|
| 1 | +# Error handling principles for the CLI |
| 2 | + |
| 3 | +* These follow from the [General Principles](https://vault.shopify.io/teams/734/pages/Error-Handling-Principles~zEv1.md) document. |
| 4 | + |
| 5 | +## Minimize the likelihood of errors happening |
| 6 | + |
| 7 | +**Errors can and will happen**, and handling them gracefully is essential to provide a first-class developer experience. |
| 8 | + |
| 9 | +When designing code, you can leverage tools to **minimize the likelihood of errors happening**. For example, you can leverage Typescript to prevent invalid states from entering the system. It requires additional time to figure out how to model the state, but the investment pays off significantly. If an external state enters the system, for example, a project in the file system, **validate it and return early** if the shape is invalid. It is similar to what web servers do with incoming HTTP requests. We also recommend modeling business logic as a combination of pure functions that are predictable. Functions with the freedom to mutate input state might get mutations introduced in the future that breaks the contract with callers of the function. |
| 10 | + |
| 11 | +Moreover, you should think about scenarios other than the happy path and build for them. There are infinite of those, especially considering that the CLI runs in environments we don't control, so it's essential that you focus on the most obvious ones and don't get too obsessed with trying to anticipate every possible error. **Excellent error management is an iterative process** using unhandled errors as input to inform the improvements. |
| 12 | + |
| 13 | +## Global type and handler for fatal errors |
| 14 | + |
| 15 | +You’ll see in the section below that the CLI currently makes an exception to some of the general principles presented in this doc. This may not be true forever since there are some upcoming projects that will change the tradeoffs. |
| 16 | + |
| 17 | +### Leverage the `FatalError` hierarchy to short circuit the program and present a custom UX to the user |
| 18 | + |
| 19 | +The CLI defines `FatalError` and several subtypes of `FatalError`. Within the CLI codebase, you can raise any of these subtypes at any time to **fully halt the program and present a specific UX to the user**. |
| 20 | + |
| 21 | +**Principle**: choose the right error type if you need to abort the program and handle the situation in a specific way: |
| 22 | + |
| 23 | +* Use `AbortError` for expected scenarios requiring user action |
| 24 | +* Use `BugError` for programmer errors and invariant violations |
| 25 | +* Use `AbortSilentError` for user-initiated cancellations |
| 26 | +* Use `ExternalError` when external commands fail |
| 27 | + |
| 28 | +Please, **don't** use the global `process.exit` and `process.abort` APIs. Also, don't `try {} catch {}` abort errors. If you need to communicate the failure of an operation to the caller (e.g., a 5xx HTTP response), use the result type from the following section. |
| 29 | + |
| 30 | +#### AbortError |
| 31 | + |
| 32 | +Represents expected user/environment issues that require user action |
| 33 | + |
| 34 | +**Characteristics:** |
| 35 | + |
| 36 | +* Most common error type in the CLI |
| 37 | +* Provides UI guidance to users |
| 38 | +* Reported as "expected\_error" to analytics (it’s a user problem, not a Shopify one) |
| 39 | +* Sent to Bugsnag as "handled" |
| 40 | + |
| 41 | +You can also pass some next steps to the error constructor to help the user recover from the error. The next steps are displayed after the error message. |
| 42 | + |
| 43 | +For example |
| 44 | +```ts |
| 45 | +throw new AbortError( |
| 46 | + "The project doesn't exist", |
| 47 | + undefined, |
| 48 | + [ |
| 49 | + "Make sure the command is executed from a project's directory", |
| 50 | + "Run the command again", |
| 51 | + ] |
| 52 | +) |
| 53 | +``` |
| 54 | + |
| 55 | +#### BugError |
| 56 | + |
| 57 | +Represents unexpected issues that indicate CLI bugs |
| 58 | + |
| 59 | +**Characteristics:** |
| 60 | + |
| 61 | +* Always reported as "unexpected\_error" and sent to Bugsnag as "unhandled" |
| 62 | +* Shows stack trace in development |
| 63 | +* Should be used for invariant violations and programmer errors |
| 64 | + |
| 65 | +#### AbortSilentError |
| 66 | + |
| 67 | +Silent termination without any output |
| 68 | + |
| 69 | +**Characteristics:** |
| 70 | + |
| 71 | +* Used when user cancels operations (e.g., declining migrations) |
| 72 | +* No message rendered to user |
| 73 | +* Clean exit without error indicators |
| 74 | + |
| 75 | +#### ExternalError |
| 76 | + |
| 77 | +Errors from external command execution |
| 78 | + |
| 79 | +**Characteristics:** |
| 80 | + |
| 81 | +* Extends AbortError with command context |
| 82 | +* Shows which external command failed |
| 83 | +* Helps users understand issues with dependencies |
| 84 | + |
| 85 | +### Divergence from general principles {#divergence-from-general-principles} |
| 86 | + |
| 87 | +The CLI currently makes an exception to a few principles, notably: |
| 88 | + |
| 89 | +- Model errors as part of your API at the same abstraction level |
| 90 | +- Don’t use exceptions for control flow; model expected states explicitly |
| 91 | + |
| 92 | +The CLI codebase generally assumes that all code is executing in context of a user’s terminal, and grants an affordance to any code in the repo where that code can immediately abort execution of the entire program and render a specific UI to the user. This is handled via `FatalError` and its descendents. |
| 93 | + |
| 94 | +An example from the app loader (where the CLI loads the user’s app from their file system) |
| 95 | + |
| 96 | +``` |
| 97 | +async function getConfigurationPath(appDirectory: string, configName: string | undefined) { |
| 98 | + const configurationFileName = getAppConfigurationFileName(configName) |
| 99 | + const configurationPath = joinPath(appDirectory, configurationFileName) |
| 100 | +
|
| 101 | + if (await fileExists(configurationPath)) { |
| 102 | + return {configurationPath, configurationFileName} |
| 103 | + } else { |
| 104 | + throw new AbortError(outputContent`Couldn't find ${configurationFileName} in ${outputToken.path(appDirectory)}.`) |
| 105 | + } |
| 106 | +} |
| 107 | +``` |
| 108 | + |
| 109 | +Our general principles might lead us to code that looks like this, raising a domain-specific error and punting the handling of that to callers. |
| 110 | + |
| 111 | +``` |
| 112 | +async function getConfigurationPath(appDirectory: string, configName: string | undefined) { |
| 113 | + const configurationFileName = getAppConfigurationFileName(configName) |
| 114 | + const configurationPath = joinPath(appDirectory, configurationFileName) |
| 115 | +
|
| 116 | + if (await fileExists(configurationPath)) { |
| 117 | + return {configurationPath, configurationFileName} |
| 118 | + } else { |
| 119 | + throw new FileNotFoundError(configurationFileName, outputToken.path(appDirectory)) |
| 120 | + } |
| 121 | +} |
| 122 | +``` |
| 123 | + |
| 124 | +Given the CLI’s existing architecture, there are some benefits to this approach. |
| 125 | + |
| 126 | +| Solution | Pros | Cons | |
| 127 | +| :---- | :---- | :---- | |
| 128 | +| `FatalError` | Can rely on a central, global handler for FatalError (and children), providing consistent UX and handling for any kind of user-visible error. Very easy UX management \- raise FatalError subtype, define UI, and it works. | Assumes the user will be able to understand and take action against an error at the current level of abstraction (i.e. that the user will understand how to action a “file not found” error). This is often a safe assumption with the CLI. All CLI services become coupled to the UI layer, making portability and testability more difficult. | |
| 129 | +| Domain-specific errors | Services code is not coupled to UI layer, which means code is more portable/testable. For service errors where the user may not be able to take action, allows delegation of handling to a higher level of abstraction where the user may be able to act more easily. | Requires more intention for handlers of domain specific errors, which could lead to UX fragmentation or inconsistent application of handling principles. | |
| 130 | + |
| 131 | +Historically we have been able to make the assumption that everything running in the CLI repo is also running in context of a CLI in a user’s terminal. And since the CLI architecturally is a fairly simple app (commands \> services with a file system for state management), the case for `FatalError` has been strong and even though it violates the general principles, it has been the right approach for the CLI. |
| 132 | + |
| 133 | +### In the future… |
| 134 | + |
| 135 | +It’s very likely that much of the code running in the CLI will need to be made more portable to different operating environments, many of which will not be running inside of a shell on a user’s machine. See [App Dev SDK and Agentic App Dev Architecture](https://docs.google.com/document/u/0/d/142iy_NKDWlTpxvuUTtIreyMd2SIAazYiXNY52VQYsiE/edit)for more information. |
| 136 | + |
| 137 | +The `FatalError` pattern will not work well in an architecture where app development activities are happening in multiple operating environments. The pros listed above will cease to be strong pros, and the cons will rear their heads. When pursuing the SDK project, we’ll likely need to address this. |
| 138 | + |
| 139 | +However, until then, we get a lot of leverage in the CLI from `FatalError`, so it can continue to exist as a high leverage counter-example to some of our general principles. |
| 140 | + |
| 141 | +## Report a result from a function |
| 142 | +There are scenarios where a function needs to inform the caller about the success or failure of the operation. For that, `@shopify/cli-kit` provides a result utility: |
| 143 | + |
| 144 | +```ts |
| 145 | +import { FatalError } from "@shopify/cli-kit/node/error" |
| 146 | +import {err, ok, Result} from '@shopify/cli-kit/node/result' |
| 147 | + |
| 148 | +class ActionError extends FatalError {} |
| 149 | + |
| 150 | +function action({success}: {success: boolean}): Result<string, ActionError> { |
| 151 | + if (success) { |
| 152 | + return ok("ok") |
| 153 | + } else { |
| 154 | + return err(new ActionError("err")) |
| 155 | + } |
| 156 | +} |
| 157 | + |
| 158 | +// OK result |
| 159 | +let result = action({success: true}) |
| 160 | +result.isErr() // false |
| 161 | +result.valueOrBug() // ok |
| 162 | +result.mapError((error) => new FatalError("other error")) |
| 163 | + |
| 164 | +// Error result |
| 165 | +let result = action({success: false}) |
| 166 | +result.isErr() // true |
| 167 | +result.valueOrBug() // throws! |
| 168 | +result.mapError((error) => new FatalError("other error")) |
| 169 | +``` |
| 170 | + |
| 171 | +## Environmental Issue Detection |
| 172 | + |
| 173 | +The CLI has a unique pattern for classifying non-fatal errors as environment issues through `shouldReportErrorAsUnexpected` and `errorMessageImpliesEnvironmentIssue`. |
| 174 | + |
| 175 | +### Environment Issues Are Expected Errors |
| 176 | + |
| 177 | +Certain error messages indicate environment problems rather than CLI bugs. Since the CLI operates in a highly volatile environment and has many dependencies (on local software like git, on the file system, etc), it’s very easy for code to fail due to some environmental or dependency issue on the user’s machine. |
| 178 | + |
| 179 | +**Principles:** |
| 180 | + |
| 181 | +- If it’s an Error, treat as unexpected *unless* errorMessageImpliesEnvironmentIssue returns true. |
| 182 | +- Rather than write extensive defensive code to catch all user system issues, we rely on a global backstop to classify environmental issues. |
| 183 | + |
| 184 | +The global handler has a function called `errorMessageImpliesEnvironmentIssue` which looks at a `environmentIssueMessages` constant to determine whether a particular failure is related to an environment issue on the user’s machine. We qualify these errors at the global backstop instead of defensively in each area of the codebase (arguably this is another violation of the general principles that works in the CLI, with much the same tradeoffs [discussed above](#divergence-from-general-principles)). |
| 185 | + |
| 186 | +``` |
| 187 | +const environmentIssueMessages = [ |
| 188 | + 'EPERM: operation not permitted, scandir', |
| 189 | + 'EPERM: operation not permitted, rename', |
| 190 | + 'EACCES: permission denied', |
| 191 | + 'EPERM: operation not permitted, symlink', |
| 192 | + 'This version of npm supports the following node versions', |
| 193 | + 'EBUSY: resource busy or locked', |
| 194 | + 'ENOTEMPTY: directory not empty', |
| 195 | + 'getaddrinfo ENOTFOUND', |
| 196 | + 'Client network socket disconnected before secure TLS connection was established', |
| 197 | + 'spawn EPERM', |
| 198 | + 'socket hang up', |
| 199 | +] |
| 200 | +``` |
| 201 | + |
| 202 | +#### When to Add to `errorMessageImpliesEnvironmentIssue`: |
| 203 | + |
| 204 | +Examples: |
| 205 | + |
| 206 | +* File System Permission Issues: Errors indicating the user lacks permissions |
| 207 | +* Network Connectivity Issues: DNS failures, socket errors, connection timeouts |
| 208 | +* Resource Contention: File locks, busy resources |
| 209 | +* External Tool Version Mismatches: npm/node version incompatibilities |
| 210 | +* Platform-Specific Issues: OS-level restrictions |
| 211 | + |
| 212 | +**Principle:** Be conservative when adding to this list. |
| 213 | + |
| 214 | +* Add only messages that are highly generic, cross-cutting, and clearly attributable to the user environment or transient OS/network—not to CLI logic. |
| 215 | +* Ensure the string is stable across Node versions/platforms; avoid brittle substrings that drift. |
| 216 | +* Prefer minimal, precise substrings that reduce false positives (e.g., include errno phrase and relevant operation). |
| 217 | +* Consider platform variance (Windows vs POSIX errors) and add both where appropriate. |
| 218 | +* Avoid domain-specific strings tied to Shopify APIs or business logic; those should be handled where they occur. |
| 219 | +* When adding, include a short rationale and links to incident/bug reports validating the classification. |
| 220 | +* Reassess entries that generate false negatives/positives using telemetry; prune aggressively if noisy. |
0 commit comments