refreshing output not stripped in error output with 'strip_refreshing' on custom run command

[rossstr-clickup]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

Overview of the Issue

#3518 Introduced a feature to filter output when running custom commands. However, if the plan workflow uses a custom run step to execute the plan, and the plan errors, the refreshing output is carried through in the error. This can lead to lengthy error comments.

This should be solvable by moving the location at which strip_refreshing is applied.

Reproduction Steps

Run plan with a custom run step, which has output: strip_refreshing set, and run a plan which results in an error.

Logs

Environment details

Additional Context

[pseudomorph]

Perhaps something like this:

https://github.com/runatlantis/atlantis/blob/main/server/core/runtime/run_step_runner.go#L72-L93

	output, err := runner.Run(ctx)

+	if postProcessOutput == valid.PostProcessRunOutputStripRefreshing {
+		output = StripRefreshingFromPlanOutput(output, tfVersion)
+	}

	if err != nil {
		err = fmt.Errorf("%s: running %q in %q: \n%s", err, command, path, output)
		if !ctx.CustomPolicyCheck {
			ctx.Log.Debug("error: %s", err)
			return "", err
		}
		ctx.Log.Debug("Treating custom policy tool error exit code as a policy failure.  Error output: %s", err)
	}

	switch postProcessOutput {
	case valid.PostProcessRunOutputHide:
		return "", nil
	case valid.PostProcessRunOutputStripRefreshing:
-		return StripRefreshingFromPlanOutput(output, tfVersion), nil
+		return output, nil
	case valid.PostProcessRunOutputShow:
		return output, nil
	default:
		return output, nil
	}

I imagine that hiding output in cases of error is undesirable.