Exit codes #20

benblank · 2018-06-18T22:42:11Z

Exit codes are an important part of Notion's communication with external tools and a first-class part of the design of its user-friendly error messages. We need a strategy for ensuring they are used and communicated consistently.

Rendered

dherman · 2018-06-28T23:00:58Z

I think the main thing I'd like to see here is separating out the following cases:

exit codes from notion
exit codes from node
exit codes from npm
exit codes from yarn
exit codes from npx and third-party executables

I would divide them up into three categories:

npm and yarn: This is where I'm most interested in getting value from exit codes, because when integrating into build/CI scripts you really want to be able to detect the main categories of errors. (Especially the network error category.) We don't control those tools but maybe we can engage with those organizations and see if there are ways they could communicate structured output (either via standard exit codes or execution modes (enabled via command-line flag or env var) that produce parseable output?) so we have a better ability to capture and categorize failures?
node, npx, and third-party executables: We have no control over these whatsoever, but if we have Notion-specific errors we still need to produce some sort of error code. How should we think about the fact that our exit codes and theirs have to share the same space of 255 values?
notion: We have total control here and can define whatever exit code scheme we want.

benblank · 2018-06-29T18:32:10Z

WRT npm and yarn, I don't like the idea of having our shims "rewrite" the tools' exit codes. Reaching out to those teams and trying to convince them of the value of good exit codes is another issue, but I think it's important that shims faithfully reproduce the behavior of the tools being shimmed, perhaps even to the point of producing a 0 exit code when the shimmed tool completes successfully even if Notion encounters errors (e.g. being unable to set up a new shim for a third-party executable after yarn installs it). If we start rewriting exit codes, we open up a whole new can of worms and the potential for misleading existing scripts which rely on those tools' current behavior.

Let me do some thinking and I'll try to update the RFC later today.

benblank · 2018-07-19T22:30:25Z

Apologies for the delay! I finally had some time to work on this and have expanded and partially rewritten the RFC.

dherman

This is awesome work, and I think I agree with basically all of it so far. I think we should go ahead and merge this RFC (unless you want to make any final minor revisions based on comments). It's still early enough that we can optimistically merge RFCs and implement them, and write followup RFCs if the design changes after we learn more from experience.

dherman · 2018-07-31T07:15:09Z

text/0000-exit-codes.md

+
+## Implementation
+
+Exit codes shall be defined as an `enum` with appropriate documentation comments and an explicit integer value for each element.  Being explicit about values allows simple, at-a-glance correlation of exit codes to error types (as well as enabling sparse values).


This is a really, really great idea. 👍

dherman · 2018-07-31T07:22:54Z

text/0000-exit-codes.md

+[unresolved]: #unresolved-questions
+
+- Is a process needed for 'vetting' new exit codes, to prevent duplication?  If so, is the RFC process appropriate?  Alternatively, is code review sufficient?
+- Should we engage npm/Yarn to improve error categorization (either through parsable output or diversifying exit codes)?


Yarn does have the ability to generate JSON formatted output, although currently you have to choose one or the other. I have reached out to some folks involved with Yarn to see if it'd be possible to "tee" the output via a config file or environment variable, so that a developer could choose whatever output they want at stdout/stderr, but we could specify a side file that also receives the output in JSON format.

dherman · 2018-07-31T07:23:07Z

text/0000-exit-codes.md

+
+## Shims
+
+Of the three "first-class" shims (`node`, `npm`, and `yarn`), only Node currently publishes a [list of exit codes](https://nodejs.org/api/process.html#process_exit_codes); the other two only emit codes 0/1 for success/failure.  The exit codes for other shimmed executables is too large a space to examine, but Node follows what appears to be a common practice: simply numbering errors starting with 1, preserving the [special meanings commonly applied by POSIX shells](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02).  This gives us incentive to avoid low-numbered exit codes (as likely to conflict) and exit codes with the high bit set (as typically representing termination due to signals).


Don't forget npx, too.

Is npx a first-class shim (i.e. does it have special handling attached to it)? I had thought it was being handled as a "generic" 3rd-party shim.

dherman · 2018-07-31T07:26:00Z

Oh, one more thing: it looks like you've done (very thorough!) research on Unix/POSIX signals, but what about Windows conventions for exit codes?

dherman · 2018-07-31T07:32:53Z

text/0000-exit-codes.md

+
+Of the three "first-class" shims (`node`, `npm`, and `yarn`), only Node currently publishes a [list of exit codes](https://nodejs.org/api/process.html#process_exit_codes); the other two only emit codes 0/1 for success/failure.  The exit codes for other shimmed executables is too large a space to examine, but Node follows what appears to be a common practice: simply numbering errors starting with 1, preserving the [special meanings commonly applied by POSIX shells](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02).  This gives us incentive to avoid low-numbered exit codes (as likely to conflict) and exit codes with the high bit set (as typically representing termination due to signals).
+
+Additionally, shims must do their utmost to preserve the exit code supplied by the shimmed executable, only introducing a shim-specific exit code if the executable cannot be run at all.  To this end, shims must return 0 when the shimmed executable does, even if the shim itself is not able to perform its work (for example, if a new shim cannot be created when installing a new executable via `npm` or `yarn`).


Really insightful point. With the idea of pushing harder on statelessness, some of these tricky edge cases may get simpler—for example, I think in the end we won't actually need npm i -g/yarn global add/etc to create shims. Generally, a more stateless model has fewer opportunities to fail, since it doesn't make as many persistent changes to the filesystem.

benblank · 2018-08-01T19:27:33Z

Oh, one more thing: it looks like you've done (very thorough!) research on Unix/POSIX signals, but what about Windows conventions for exit codes?

I added a section addressing Windows exit codes and why I'm not proposing compatibility with them.

dherman · 2018-08-16T07:01:50Z

This is looking great and looks ready to merge to me. If no one raises any issues in the next couple days I'll merge it.

Propose RFC for exit codes.

d8b5d94

Expand based on PR feedback.

872b4bd

dherman approved these changes Jul 31, 2018

View reviewed changes

dherman reviewed Jul 31, 2018

View reviewed changes

mikrostew mentioned this pull request Aug 1, 2018

exit codes volta-cli/volta#49

Closed

Add a section specifically addressing Windows exit codes.

8dfe8ef

benblank mentioned this pull request Aug 13, 2018

Exit codes volta-cli/volta#114

Merged

dherman merged commit e5ec564 into volta-cli:master Aug 21, 2018

benblank deleted the exit-codes branch August 22, 2018 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exit codes #20

Exit codes #20

benblank commented Jun 18, 2018

dherman commented Jun 28, 2018 •

edited

Loading

benblank commented Jun 29, 2018

benblank commented Jul 19, 2018

dherman left a comment

dherman Jul 31, 2018

dherman Jul 31, 2018

dherman Jul 31, 2018

benblank Aug 1, 2018

dherman commented Jul 31, 2018

dherman Jul 31, 2018

benblank commented Aug 1, 2018

dherman commented Aug 16, 2018


		## Implementation

		Exit codes shall be defined as an `enum` with appropriate documentation comments and an explicit integer value for each element. Being explicit about values allows simple, at-a-glance correlation of exit codes to error types (as well as enabling sparse values).


		## Shims

		Of the three "first-class" shims (`node`, `npm`, and `yarn`), only Node currently publishes a [list of exit codes](https://nodejs.org/api/process.html#process_exit_codes); the other two only emit codes 0/1 for success/failure. The exit codes for other shimmed executables is too large a space to examine, but Node follows what appears to be a common practice: simply numbering errors starting with 1, preserving the [special meanings commonly applied by POSIX shells](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02). This gives us incentive to avoid low-numbered exit codes (as likely to conflict) and exit codes with the high bit set (as typically representing termination due to signals).


		Of the three "first-class" shims (`node`, `npm`, and `yarn`), only Node currently publishes a [list of exit codes](https://nodejs.org/api/process.html#process_exit_codes); the other two only emit codes 0/1 for success/failure. The exit codes for other shimmed executables is too large a space to examine, but Node follows what appears to be a common practice: simply numbering errors starting with 1, preserving the [special meanings commonly applied by POSIX shells](http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02). This gives us incentive to avoid low-numbered exit codes (as likely to conflict) and exit codes with the high bit set (as typically representing termination due to signals).

		Additionally, shims must do their utmost to preserve the exit code supplied by the shimmed executable, only introducing a shim-specific exit code if the executable cannot be run at all. To this end, shims must return 0 when the shimmed executable does, even if the shim itself is not able to perform its work (for example, if a new shim cannot be created when installing a new executable via `npm` or `yarn`).

Exit codes #20

Exit codes #20

Conversation

benblank commented Jun 18, 2018

dherman commented Jun 28, 2018 • edited Loading

benblank commented Jun 29, 2018

benblank commented Jul 19, 2018

dherman left a comment

Choose a reason for hiding this comment

dherman Jul 31, 2018

Choose a reason for hiding this comment

dherman Jul 31, 2018

Choose a reason for hiding this comment

dherman Jul 31, 2018

Choose a reason for hiding this comment

benblank Aug 1, 2018

Choose a reason for hiding this comment

dherman commented Jul 31, 2018

dherman Jul 31, 2018

Choose a reason for hiding this comment

benblank commented Aug 1, 2018

dherman commented Aug 16, 2018

dherman commented Jun 28, 2018 •

edited

Loading