Skip to content

protocol-definitions_v2.0.0

@znewton znewton tagged this 14 Jul 17:38
## Description

We have been seeing many Uncaught Exceptions in lambdas like Deli and
Scribe. From a code perspective, it looks like all of these locations
are wrapped in try..catch blocks or .then.catch chains. However,
sometimes these errors are getting bubbled up as uncaught and causing
the services to restart.

### Change 1: make sure optionalPromise then/catch are set immediately

Coming up to the top of the stack, we see in DocumentPartition and
Partition classes that we don't immediately set `.catch` on the lambda
handler promise. I think it's possible this could be contributing, and
it doesn't hurt to change it.

### Change 2: then(success, error) vs .then().catch()

Using `then(success, error)` does not handle errors thrown in the
success handler. For example, the following will throw an uncaught
exception.
```ts
myPromise.then(() => { throw new Error()}, (error) => console.error(error));
```
This pattern is used in a couple places in the R11s codebase. I have
added the ESLint rule
[promise/catch-or-return](https://github.com/eslint-community/eslint-plugin-promise/blob/main/docs/rules/catch-or-return.md#enforce-the-use-of-catch-on-un-returned-promises-promisecatch-or-return)
to make sure this never happens again.
Assets 2
Loading