Skip to content

Commit

Permalink
chore(guidelines): update the section on error messages (aws#13345)
Browse files Browse the repository at this point in the history
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored and cornerwings committed Mar 8, 2021
1 parent 4cb8e6c commit 52f29fa
Showing 1 changed file with 107 additions and 20 deletions.
127 changes: 107 additions & 20 deletions DESIGN_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -1261,19 +1261,6 @@ for (const az of availabilityZones) {
### Errors
#### Input Validation
Prefer to validate input as early as it is passed into your code (ctor, methods,
etc) and bail out by throwing an **Error** (no need to create subclasses of
Error since all errors in the CDK are unrecoverable):
* All lowercase sentences (usually they are printed after “Error: \<message\>”)
* Include a descriptive message
* Include the value provided
* Include the expected/allowed values
* No need to include information that can be obtained from the stack trace
* No need to add a period at the end of error messages
#### Avoid Errors If Possible
Always prefer to do the right thing for the user instead of raising an
Expand All @@ -1282,26 +1269,126 @@ example, VPC has **enableDnsHostnames** and **enableDnsSupport**. DNS hostnames
*require* DNS support, so only fail if the user enabled DNS hostnames but
explicitly disabled DNS support. Otherwise, auto-enable DNS support for them.
#### Error reporting mechanism
There are three mechanism you can use to report errors:
* Eagerly throw an exception (fails synthesis)
* Attach a (lazy) validator to a construct (fails synthesis)
* Attach errors to a construct (succeeds synthesis, fails deployment)
Between these, the first two fail synthesis, while the latter doesn't. Failing synthesis
means that no Cloud Assembly will be produced.
The distinction becomes apparent when you consider multiple stacks in the same Cloud
Assembly:
* If synthesis fails due to an error in *one* stack (either by throwing an exception
or by failing validation), the other stack can also not be deployed.
* In contrast, if you attach an error to a construct in one stack, that stack cannot
be deployed but the other one still can.
Choose one of the first two methods if the failure is caused by a misuse of the API,
which the user should be alerted to and fix as quickly as possible. Choose attaching
an error to a construct if the failure is due to environmental factors outside the
direct use of the API surface (for example, lack of context provider lookup values).
#### Throwing exceptions
This should be the preferred error reporting method.
Validate input as early as it is passed into your code (ctor, methods,
etc) and bail out by throwing an `Error`. No need to create subclasses of
Error since all errors in the CDK are unrecoverable.
When validating inputs, don't forget to account for the fact that these
values may be `Token`s and not available for inspection at synthesis time.
Example:
```ts
if (!Token.isUnresolved(props.minCapacity) && props.minCapacity < 1) {
throw new Error(`'minCapacity' should be at least 1, got '${props.minCapacity}'`);
}
```
#### Never Catch Exceptions
All CDK errors are unrecoverable. If a method wishes to signal a recoverable
All CDK errors are unrecoverable. If a method wishes to signal a recoverable
error, this should be modeled in a return value and not through exceptions.
#### Post Validation
#### Attaching (lazy) Validators
In the rare case where the integrity of your construct can only be checked right
before synthesis, override the **Construct.validate()** method and return
meaningful errors. Always prefer early input validation over post-validation.
In the rare case where the integrity of your construct can only be checked
after the app has completed its initialization, call the
**this.node.addValidation()** method to add a validation object. This will
generally only be necessary if you want to produce an error when a certain
interaction with your construct did *not* happen (for example, a property
that should have been configured over the lifetime of the construct, wasn't):
#### Attached Errors/Warnings
Always prefer early input validation over post-validation, as the necessity
of these should be rare.
Example:
```ts
this.node.addValidation({
// 'validate' should return a string[] list of errors
validate: () => this.rules.length === 0
? ['At least one Rule must be added. Call \'addRule()\' to add Rules.']
: []
}
});
```
#### Attaching Errors/Warnings
You can also “attach” an error or a warning to a construct via
the **Annotations** class. These methods (e.g., `Annotations.of(construct).addWarning`)
will attach CDK metadata to your construct, which will be displayed to the user
by the toolchain when the stack is deployed.
Errors will not allow deployment and warnings will only be displayed in
highlight (unless **--strict** mode is used).
highlight (unless `--strict` mode is used).
```ts
if (!Token.isUnresolved(subnetIds) && subnetIds.length < 2) {
Annotations.of(this).addError(`Need at least 2 subnet ids, got: ${JSON.stringify(subnetIds)}`);
}
```
#### Error messages
Think about error messages from the point of view of the end user of the CDK.
This is not necessarily someone who knows about the internals of your
construct library, so try to phrase the message in a way that would make
sense to them.
For example, if a value the user supplied gets handed off between a number of
functions before finally being validated, phrase the message in terms of the
API the user interacted with, not in terms of the internal APIs.
A good error message should include the following components:
* What went wrong, in a way that makes sense to a top-level user
* An example of the incorrect value provided (if applicable)
* An example of the expected/allowed values (if applicable)
* The message should explain the (most likely) cause and change the user can
make to rectify the situation
The message should be all lowercase and not end in a period, or contain
information that can be obtained from the stack trace.
```ts
// ✅ DO - show the value you got and be specific about what the user should do
`supply at least one of minCapacity or maxCapacity, got ${JSON.stringify(action)}`

// ❌ DO NOT - this tells the user nothing about what's wrong or what they should do
`required values are missing`

// ❌ DO NOT - this error only makes sense if you know the implementation
`'undefined' is not a number`
```
### Tokens
Expand Down

0 comments on commit 52f29fa

Please sign in to comment.