Change heroku run -x
implementation to use a Bash EXIT trap
#2468
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
When
-x
is passed toheroku run
, the given command is suffixed with a specialecho
that returns$?
, the exit status of the program:cli/packages/run-v5/lib/dyno.js
Line 54 in b451c6b
This
\uFFFF heroku-command-exit-status: …
output is later detected and used:cli/packages/run-v5/lib/dyno.js
Lines 315 to 328 in b451c6b
Exit status reporting issues
Appending our own
; echo …
to the command causes several bugs:exit
statement (simple example:heroku run -x 'exit 3'
):bash -c
instead, as a workaround:set -e
orset -u
:heroku run -x 'echo hello;'
):heroku logs
- it's always 0 (from theecho
), not the correct value:Exit status reporting solution
Using a
trap
onEXIT
instead works around all of these issues (in this example, the construction is a bit convoluted due to escaping challenges, but the principle is the same as the contents of this PR):Magic string handling issues
Additionally, there are problems with the matching and removal of the magic
\uFFFF heroku-command-exit-status: …
string.First, the magic string replace anchors on the beginning of a line, but if the last line of output does not end with a newline, this will fail:
Second, it always leaves behind a newline character. This means the output between
heroku run
andheroku run -x
differs:The reason is that the
\n?
is ungreedy due to the?
, so it will not actually match the newline character.Magic string handling solution
The anchoring at the beginning of the line is easy to remove in the
replace
Regex.For the trailing newline, the solution is to not have the newline optional in the Regex (our
echo
always prints it, after all), or to use a different trailing marker - I have re-used\uFFFF
here, because that also allows removing multiline mode.Ideally, we'd use a proper unique string - e.g. generate a GUID for the command, and match that exact GUID later. Could also use a different separator -
\uFFFF
is not legal unicode, but 💜 is ;) (that's\U1F49C
in Bash, with uppercase "U
", and\uD83D\uDC9C
in JavaScript, expressed as a UTF-16 surrogate pair).