From ce1e12bdbea2c07e49df35292c4368379406d667 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Tue, 28 Apr 2020 15:51:28 -0700 Subject: [PATCH 01/15] doc: add unhandled promise rejection survey Signed-off-by: Matheus Marchini --- surveys/promise-rejections/survey.md | 68 ++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 surveys/promise-rejections/survey.md diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md new file mode 100644 index 00000000..112ee0e8 --- /dev/null +++ b/surveys/promise-rejections/survey.md @@ -0,0 +1,68 @@ +# Node.js Promise reject use case survey + +## Are you currently using Promises, async/await, a mix, or neither? + +Assume using as creating your own `new Promise` and `async function` + + - [ ] Promises + - [ ] async/await + - [ ] Both + - [ ] Neither / not writing Promise-based code + +## Are you currently using `.catch()`, `try await catch`, a mix, or neither to handle rejections? + +When consuming Promises, async functions or thenables, which of the options below do you use to handle rejections? + + - [ ] `.catch()` + - [ ] `try await catch` + - [ ] Both + - [ ] Neither / not writing Promise-based code + +## Do you use one or more of the following options as a global handler for Promise rejections? + +Leave empty if you use the default Node.js behavior + + - [ ] Yes, via process.on('unhandledRejection') + - [ ] Yes, via the `--unhandled-rejections` flag + +## Are you using Promises in customer facing, production applications? + + - [ ] Yes + - [ ] No + - [ ] I don't know + +## Which of the use cases described below do you spend most of your time on? + + - [ ] Third-party libraries + - [ ] Command-line tools + - [ ] Web servers + - [ ] Computational-heave processing + - [ ] Desktop applications + - [ ] Other (please elaborate) + +## When a rejected promise doesn't have a catch handler, what is the behavior of your application today? + + - [ ] Current default Node.js behavior: logs a warning alongside a deprecation notice, continue execution + - [ ] Logs a warning, continue running, no deprecation warning + - [ ] Logs a warning, continue running, no deprecation warning, exits with an error code when program finishes + - [ ] Exit as soon as possible + - [ ] Other (please elaborate) + +## What should be the default Node.js behavior for unhandled rejections? + +Consider the following modes: + + - `strict`: throw and crash as soon as possible (on nextTick). Can't be overriden by unhandledRejection listener + - `throw`: throw and crash as soon as possible (on nextTick). Can be overriden by unhandledRejection listener + - `warn`: outputs a warning as soon as possible (on nextTick). Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with a success code + - `warn-with-error`: outputs a warning as soon as possible (on nextTick). Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with an error code + - `none`: do nothing + +Which one you think should be the default on Node.js? + + - [ ] `strict` + - [ ] `throw` + - [ ] `warn` + - [ ] `warn-with-error` + - [ ] `none` + - [ ] Other (please elaborate) From 69ae96b0aa0592aedd320c6061eb6798b729158c Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 29 Apr 2020 10:53:13 -0700 Subject: [PATCH 02/15] fixup! doc: add unhandled promise rejection survey Co-Authored-By: Rich Trott --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 112ee0e8..29d78d8f 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -36,7 +36,7 @@ Leave empty if you use the default Node.js behavior - [ ] Third-party libraries - [ ] Command-line tools - [ ] Web servers - - [ ] Computational-heave processing + - [ ] Computation-heavy processing - [ ] Desktop applications - [ ] Other (please elaborate) From 4eea2a649b3f5738fde4738bb168a2a25f64fca3 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 29 Apr 2020 10:53:27 -0700 Subject: [PATCH 03/15] fixup! doc: add unhandled promise rejection survey Co-Authored-By: Andrey Pechkurov <37772591+puzpuzpuz@users.noreply.github.com> --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 29d78d8f..69e1be0d 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -22,7 +22,7 @@ When consuming Promises, async functions or thenables, which of the options belo Leave empty if you use the default Node.js behavior - - [ ] Yes, via process.on('unhandledRejection') + - [ ] Yes, via `process.on('unhandledRejection')` - [ ] Yes, via the `--unhandled-rejections` flag ## Are you using Promises in customer facing, production applications? From ac30722b47e982d7b228a74ee5d2f8bd33d4dc03 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 29 Apr 2020 10:53:41 -0700 Subject: [PATCH 04/15] !fixup doc: add unhandled promise rejection survey Co-Authored-By: James M Snell --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 69e1be0d..f88a5709 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -42,7 +42,7 @@ Leave empty if you use the default Node.js behavior ## When a rejected promise doesn't have a catch handler, what is the behavior of your application today? - - [ ] Current default Node.js behavior: logs a warning alongside a deprecation notice, continue execution + - [ ] Node.js logs a warning alongside a deprecation notice, execution continues - [ ] Logs a warning, continue running, no deprecation warning - [ ] Logs a warning, continue running, no deprecation warning, exits with an error code when program finishes - [ ] Exit as soon as possible From 892f43d6eb7be8e27b22cc4f057658e0777410a1 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 6 May 2020 11:24:50 -0700 Subject: [PATCH 05/15] fixup! doc: add unhandled promise rejection survey --- surveys/promise-rejections/survey.md | 38 ++++++++++++++++++---------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index f88a5709..0b0ae873 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -4,26 +4,32 @@ Assume using as creating your own `new Promise` and `async function` - - [ ] Promises - - [ ] async/await - - [ ] Both - - [ ] Neither / not writing Promise-based code +(Multiple choice) -## Are you currently using `.catch()`, `try await catch`, a mix, or neither to handle rejections? + - [ ] Promises + - [ ] async/await + - [ ] Not writing Promise-based code + +## How are you handling rejections today? When consuming Promises, async functions or thenables, which of the options below do you use to handle rejections? - - [ ] `.catch()` - - [ ] `try await catch` - - [ ] Both - - [ ] Neither / not writing Promise-based code +(Multiple choice) + + - [ ] `.catch()` + - [ ] `try / catch` wrapping an `await` operation + - [ ] Ignore the rejection + - [ ] Not writing Promise-based code -## Do you use one or more of the following options as a global handler for Promise rejections? +## Do you use `process.on('unhandledRejection')` as a global handler for unhandled rejections? + + - [ ] Yes + - [ ] No -Leave empty if you use the default Node.js behavior +## Do you use the `--unhandled-rejections` command-line flag as a global handler for unhandled rejections? - - [ ] Yes, via `process.on('unhandledRejection')` - - [ ] Yes, via the `--unhandled-rejections` flag + - [ ] Yes + - [ ] No ## Are you using Promises in customer facing, production applications? @@ -66,3 +72,9 @@ Which one you think should be the default on Node.js? - [ ] `warn-with-error` - [ ] `none` - [ ] Other (please elaborate) + +# Are you aware that not handling rejections may cause memory leaks or denial of service vulnerabilities? + + - [ ] Yes + - [ ] No + - [ ] Unhandled rejections do not cause memory leaks or denial of service vulnerabilities From 0c38bbca98b67b2db5817d0e4d15d6301d467500 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 6 May 2020 11:56:57 -0700 Subject: [PATCH 06/15] fixup! doc: add unhandled promise rejection survey Co-authored-by: Jordan Harband --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 0b0ae873..dea272fe 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -2,7 +2,7 @@ ## Are you currently using Promises, async/await, a mix, or neither? -Assume using as creating your own `new Promise` and `async function` +Assume using as creating your own `new Promise`, `Promise.resolve`, and `async function`; or using libraries that produce Promises (that you use with `.then`/`.catch`, or with `.await`) (Multiple choice) From 3563cceec3e074cc030b52ec31496bbee362716e Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 6 May 2020 11:57:11 -0700 Subject: [PATCH 07/15] fixup! doc: add unhandled promise rejection survey Co-authored-by: Jordan Harband --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index dea272fe..2296d4c7 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -8,7 +8,7 @@ Assume using as creating your own `new Promise`, `Promise.resolve`, and `async f - [ ] Promises - [ ] async/await - - [ ] Not writing Promise-based code + - [ ] Not writing Promise-based code or using Promise-based libraries ## How are you handling rejections today? From 39fe16780280e6f73fddaa85c8cc5ba15ff77a8c Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 6 May 2020 11:57:21 -0700 Subject: [PATCH 08/15] fixup! doc: add unhandled promise rejection survey Co-authored-by: Jordan Harband --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 2296d4c7..cf82dea0 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -19,7 +19,7 @@ When consuming Promises, async functions or thenables, which of the options belo - [ ] `.catch()` - [ ] `try / catch` wrapping an `await` operation - [ ] Ignore the rejection - - [ ] Not writing Promise-based code + - [ ] Not writing Promise-based code or using Promise-based libraries ## Do you use `process.on('unhandledRejection')` as a global handler for unhandled rejections? From 84fe43e2c44f75d379663df3d2c513d9fddfed7a Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 13 May 2020 17:56:45 -0700 Subject: [PATCH 09/15] fixup! fixup! doc: add unhandled promise rejection survey --- surveys/promise-rejections/survey.md | 96 ++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index cf82dea0..42a85669 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -1,5 +1,70 @@ # Node.js Promise reject use case survey +Today, Node.js handles unhandled rejections by emitting a deprecation warning to stderr. The warning shows the stack where the rejection happened, and states that in future Node.js versions unhandled rejections will result on Node.js exitting with non-zero status code. We intend to remove the deprecation warning, replacing it with a stable behavior which might be different from the one described on the deprecation warning. We're running this survey to better understand how Node.js users are using Promises and how they are dealing with unhandled rejections today, so we can make an informed decision on how to move forward. + +## What is a Promise rejection? + +A Promise rejection indicates that something went wrong while executing a Promise or an async function. Rejections can occur on several situations: thorwing inside an async function or a `Promise` executor/then/catch/finally callback, when calling the `reject` callback of an `executor`, or when calling `Promise.reject`. + +```js + +Promise.reject() // This will result in a rejection + +new Promise((fulfill, reject) => { + reject(); // This will result in a rejection +}); + +new Promise(() => { + throw new Error(); // This will result in a rejection +}).then(() => { + throw new Error(); // This will result in a rejection +}, () => { + throw new Error(); // This will result in a rejection +}).catch(() => { + throw new Error(); // This will result in a rejection +}).finally(() => { + throw new Error(); // This will result in a rejection +}); + +async function () { + throw new Error(); // This will result in a rejection +} + +async function () { + if (a === undefined) + a(); // This will result in a rejection +} +``` + +## What is an unhandled rejection? + +Rejections are designed to be handled asynchronously. In JavaScript, Promises can be handled when awaiting for a promise inside a try/catch block, or via .catch calls: + + +```js +async function foo() { + throw new Error(); +} + +foo().catch(() => console.error("an error occured")); // 1. Handled + +try { + await foo(); // 2. Handled +} catch(e) { + console.error("an error occured") +} + +foo(); // 3. Unhandled, but execution continues + +const rejected = foo(); // 4. Unhandled on current event loop tick + +// 4. ... but handled asynchronously when the setTimeout callback is popped from +// the event loop +setTimeout(() => rejected.catch(() => console.error("an error occured")), 100); +``` + +An unhandled rejection is a rejection which hasn't been handled yet. It might be handled in the future, like example 4, but it might also stay unhandled forever (like example 3). + ## Are you currently using Promises, async/await, a mix, or neither? Assume using as creating your own `new Promise`, `Promise.resolve`, and `async function`; or using libraries that produce Promises (that you use with `.then`/`.catch`, or with `.await`) @@ -21,15 +86,19 @@ When consuming Promises, async functions or thenables, which of the options belo - [ ] Ignore the rejection - [ ] Not writing Promise-based code or using Promise-based libraries -## Do you use `process.on('unhandledRejection')` as a global handler for unhandled rejections? +## Do you know that Node.js has a global handled for unhandled rejections (`process.on('unhandledRejection')`)? If so, do you use it? - - [ ] Yes - - [ ] No + - [ ] I use `process.on('unhandledRejection')` + - [ ] I don't use `process.on('unhandledRejection')` + - [ ] I didn't know `process.on('unhandledRejection')` existed -## Do you use the `--unhandled-rejections` command-line flag as a global handler for unhandled rejections? +## Do you know that Node.js has a flag to change the behavior of unhandled rejections `--unhandled-rejections`? If so, do you use it? - - [ ] Yes - - [ ] No + - [ ] I use `--unhandled-rejections` set to `'strict'` + - [ ] I use `--unhandled-rejections` set to `'warn'` + - [ ] I use `--unhandled-rejections` set to `'none'` + - [ ] I don't use `--unhandled-rejections` + - [ ] I didn't know `--unhandled-rejections` existed ## Are you using Promises in customer facing, production applications? @@ -48,6 +117,15 @@ When consuming Promises, async functions or thenables, which of the options belo ## When a rejected promise doesn't have a catch handler, what is the behavior of your application today? +For example: + +```js +async function error() { + throw new Error(); +} +error(); +``` + - [ ] Node.js logs a warning alongside a deprecation notice, execution continues - [ ] Logs a warning, continue running, no deprecation warning - [ ] Logs a warning, continue running, no deprecation warning, exits with an error code when program finishes @@ -72,9 +150,3 @@ Which one you think should be the default on Node.js? - [ ] `warn-with-error` - [ ] `none` - [ ] Other (please elaborate) - -# Are you aware that not handling rejections may cause memory leaks or denial of service vulnerabilities? - - - [ ] Yes - - [ ] No - - [ ] Unhandled rejections do not cause memory leaks or denial of service vulnerabilities From 69e1e5a4052e52befb7cd361453c37195f08a995 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Thu, 14 May 2020 14:56:20 -0700 Subject: [PATCH 10/15] fixup! Co-authored-by: Jordan Harband --- surveys/promise-rejections/survey.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 42a85669..abe302ea 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -49,8 +49,8 @@ async function foo() { foo().catch(() => console.error("an error occured")); // 1. Handled try { - await foo(); // 2. Handled -} catch(e) { + await foo(); +} catch(e) { // 2. Handled console.error("an error occured") } From a7a8e68a8085d2c2175b985435a3199eeadc0f52 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Thu, 14 May 2020 14:58:54 -0700 Subject: [PATCH 11/15] fixup! Co-authored-by: Gus Caplan --- surveys/promise-rejections/survey.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index abe302ea..011c6712 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -67,7 +67,7 @@ An unhandled rejection is a rejection which hasn't been handled yet. It might be ## Are you currently using Promises, async/await, a mix, or neither? -Assume using as creating your own `new Promise`, `Promise.resolve`, and `async function`; or using libraries that produce Promises (that you use with `.then`/`.catch`, or with `.await`) +Assume using as creating your own `new Promise`, `Promise.resolve`, and `async function`; or using libraries that produce Promises (that you use with `.then`/`.catch`/`.finally`, or with `await`) (Multiple choice) From 5de901427a6a01e89162cbde75f121803d889f2e Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Thu, 14 May 2020 17:06:39 -0700 Subject: [PATCH 12/15] fixup! fixup! fixup! doc: add unhandled promise rejection survey --- surveys/promise-rejections/survey.md | 127 ++++++++++++++++++++------- 1 file changed, 93 insertions(+), 34 deletions(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 011c6712..342afd3b 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -1,28 +1,35 @@ # Node.js Promise reject use case survey -Today, Node.js handles unhandled rejections by emitting a deprecation warning to stderr. The warning shows the stack where the rejection happened, and states that in future Node.js versions unhandled rejections will result on Node.js exitting with non-zero status code. We intend to remove the deprecation warning, replacing it with a stable behavior which might be different from the one described on the deprecation warning. We're running this survey to better understand how Node.js users are using Promises and how they are dealing with unhandled rejections today, so we can make an informed decision on how to move forward. +Today, Node.js handles unhandled rejections by emitting a deprecation warning to stderr. The warning shows the stack where the rejection happened, and states that in future Node.js versions unhandled rejections will result in Node.js exiting with non-zero status code. We intend to remove the deprecation warning, replacing it with a stable behavior which might be different from the one described on the deprecation warning. We're running this survey to better understand how Node.js users are using Promises and how they are dealing with unhandled rejections today, so we can make an informed decision on how to move forward. ## What is a Promise rejection? -A Promise rejection indicates that something went wrong while executing a Promise or an async function. Rejections can occur on several situations: thorwing inside an async function or a `Promise` executor/then/catch/finally callback, when calling the `reject` callback of an `executor`, or when calling `Promise.reject`. +A Promise rejection indicates that something went wrong while executing a Promise or an async function. Rejections can occur in several situations: throwing inside an async function or a `Promise` executor/then/catch/finally callback, when calling the `reject` callback of an `executor`, or when calling `Promise.reject`. ```js - -Promise.reject() // This will result in a rejection +Promise.reject(new Error()) // This will result in a rejection new Promise((fulfill, reject) => { - reject(); // This will result in a rejection + reject(new Error()); // This will result in a rejection }); new Promise(() => { throw new Error(); // This will result in a rejection -}).then(() => { +}) + +Promise.resolve().then(() => { throw new Error(); // This will result in a rejection -}, () => { +}) + +Promise.reject().then(() = {}, () => { throw new Error(); // This will result in a rejection -}).catch(() => { +} + +Promise.reject().catch(() => { throw new Error(); // This will result in a rejection -}).finally(() => { +}) + +Promise.resolve().finally(() => { throw new Error(); // This will result in a rejection }); @@ -36,43 +43,58 @@ async function () { } ``` +Adding the `async` keyword to a function will turn any exceptions thrown inside that function (or any throw propagated from other functions called within it) into a rejection. The same happens when refactoring callback based code that throws into async functions / Promises. Below is an example of a callback based code refactored to Promises where exceptions become rejections: + ## What is an unhandled rejection? -Rejections are designed to be handled asynchronously. In JavaScript, Promises can be handled when awaiting for a promise inside a try/catch block, or via .catch calls: +There are two ways to handle rejections: by attaching a `.catch` handler to it, +or by awaiting on the promise within a try/catch block. In both cases, the +handling of the rejection (the execution of the callback passed to `.catch`, or +the execution of the `catch {}` block) will happen in a future turn of the +event loop. +Promises are designed so that attaching handlers or awaiting can be done at any +point in time, from when the Promise was created (possibly while it's still +pending), to right before the program finishes execution. + +A rejection is considered unhandled from the point it happens until the point +where a handler is attached to the Promise or the Promise is awaited within a +`catch {}` block. Below are a few examples of handled and unhandled rejections. ```js async function foo() { throw new Error(); } -foo().catch(() => console.error("an error occured")); // 1. Handled +foo() // 1. Unhandled at this point + .catch(() => console.error("an error occured")); // 2. Now it's handled try { await foo(); -} catch(e) { // 2. Handled +} catch(e) { // 3. Handled console.error("an error occured") } -foo(); // 3. Unhandled, but execution continues +foo(); // 4. Unhandled, but execution continues -const rejected = foo(); // 4. Unhandled on current event loop tick +const rejected = foo(); // 5. Unhandled on current event loop turn -// 4. ... but handled asynchronously when the setTimeout callback is popped from -// the event loop +// 4. ... but handled in a future turn of the event loop when the setTimeout +// callback is executed. setTimeout(() => rejected.catch(() => console.error("an error occured")), 100); ``` -An unhandled rejection is a rejection which hasn't been handled yet. It might be handled in the future, like example 4, but it might also stay unhandled forever (like example 3). +As we can see in the examples, an unhandled rejection might be handled in the future, like example 4, but it might also stay unhandled forever (like example 5). -## Are you currently using Promises, async/await, a mix, or neither? +Certain unhandled rejections may in rare cases leave your application in a non-deterministic and unsafe state, whether it's internal application state (including memory leaks), external resources used by your application (say, file handles or database connections), or external state (say, consistency of data in a database). -Assume using as creating your own `new Promise`, `Promise.resolve`, and `async function`; or using libraries that produce Promises (that you use with `.then`/`.catch`/`.finally`, or with `await`) +## Are you currently using Promises, async functions, a mix, or neither? (Multiple choice) - - [ ] Promises - - [ ] async/await + - [ ] Native Promises (`new Promise`, `Promise.resolve`, `Promise.reject`) + - [ ] Third-party Promises (for example: Bluebird) + - [ ] async functions - [ ] Not writing Promise-based code or using Promise-based libraries ## How are you handling rejections today? @@ -83,10 +105,10 @@ When consuming Promises, async functions or thenables, which of the options belo - [ ] `.catch()` - [ ] `try / catch` wrapping an `await` operation - - [ ] Ignore the rejection + - [ ] Leave the handling to my caller - [ ] Not writing Promise-based code or using Promise-based libraries -## Do you know that Node.js has a global handled for unhandled rejections (`process.on('unhandledRejection')`)? If so, do you use it? +## Do you know that Node.js has a global handler for unhandled rejections (`process.on('unhandledRejection')`)? If so, do you use it? - [ ] I use `process.on('unhandledRejection')` - [ ] I don't use `process.on('unhandledRejection')` @@ -98,15 +120,26 @@ When consuming Promises, async functions or thenables, which of the options belo - [ ] I use `--unhandled-rejections` set to `'warn'` - [ ] I use `--unhandled-rejections` set to `'none'` - [ ] I don't use `--unhandled-rejections` + - [ ] I use a third-party library (like make-promises-safe) to deal with unhandled rejections - [ ] I didn't know `--unhandled-rejections` existed -## Are you using Promises in customer facing, production applications? - - [ ] Yes - - [ ] No - - [ ] I don't know +## Do you know that Node.js has a global handler for uncaught exception (`process.on('uncaughtException')`)? If so, do you use it? + + - [ ] I use `process.on('unhandledRejection')` + - [ ] I don't use `process.on('unhandledRejection')` + - [ ] I didn't know `process.on('unhandledRejection')` existed + +Are you using Promises in any of the following kinds of applications? (check all that apply) -## Which of the use cases described below do you spend most of your time on? + - [ ] Production-level code + - [ ] Tests + - [ ] Benchmarks + - [ ] Build pipeline / infrastructure + - [ ] Examples and Demos + - [ ] Other (please describe) + +## Which of the use cases described below have you spent most of your time on? - [ ] Third-party libraries - [ ] Command-line tools @@ -126,22 +159,48 @@ async function error() { error(); ``` - - [ ] Node.js logs a warning alongside a deprecation notice, execution continues +If working on multiple applications or projects, choose the answer that describes the behavior on your biggest project, or on the project you worked the most. + + - [ ] Node.js logs a warning alongside a deprecation notice, execution continues (this is the default Node.js behavior today) - [ ] Logs a warning, continue running, no deprecation warning - [ ] Logs a warning, continue running, no deprecation warning, exits with an error code when program finishes - - [ ] Exit as soon as possible + - [ ] Exit as soon as possible (this is the default Node.js behavior for uncaught exceptions today) - [ ] Other (please elaborate) +## How do you deal with managing resources wrapped with promises when an unhandled rejection occurs? + +Node.js code may wrap resource managing code in async/await: + +```js +// if this method wasn't async, node would crash by default +myEmitter.on('event', async () =>{ + // react to event + databaseConnection.release(); // throws an error +}); +``` + +If the above code throws, a database handle might leak which can in some cases eventually lead to a cascading failure and a denial of service. + +How do tools or servers you author deal with this case? + + - [ ] I don't author code where this might be an issue (for example, code I author does not connect to third party resources). + - [ ] I take extra care to deal with these cases individually and perform monitoring on database handles with alerts. + - [ ] I make sure to restart my server if code like `databaseConnection.release` a throws, like with `uncaughtException`s. + - [ ] In theory this can be an issue with code I author but in practice things have been working out fine for me and I ignore these errors. + - [ ] The server keeps running, I log uncaught exceptions and unhandled rejections to monitoring and use a tool to notify engineers if such a bug occurs. + ## What should be the default Node.js behavior for unhandled rejections? Consider the following modes: - - `strict`: throw and crash as soon as possible (on nextTick). Can't be overriden by unhandledRejection listener - - `throw`: throw and crash as soon as possible (on nextTick). Can be overriden by unhandledRejection listener - - `warn`: outputs a warning as soon as possible (on nextTick). Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with a success code - - `warn-with-error`: outputs a warning as soon as possible (on nextTick). Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with an error code + - `strict`: raise an uncaught exception similar to `throw new Error()` that is not caught. `unhandledRejection` listeners do not prevent raising the exception + - `throw`: raise an uncaught exception similar to `throw new Error()` that is not caught. `unhandledRejection` listeners take precedence and prevent raising the exception + - `warn`: outputs a warning as soon as possible. Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with a success code. This is similar to what browser consoles do + - `warn-with-error`: outputs a warning as soon as possible. Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with an error code - `none`: do nothing +For all the modes, the action (raise an exceptiom output a warninig) will happen on `nextTick`. + Which one you think should be the default on Node.js? - [ ] `strict` From 5ccb7797bd973faf9b0a456c7c8642388973597a Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Tue, 19 May 2020 12:37:20 -0700 Subject: [PATCH 13/15] fixup! fixup! fixup! fixup! doc: add unhandled promise rejection survey --- surveys/promise-rejections/survey.md | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 342afd3b..239d2d13 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -88,6 +88,30 @@ As we can see in the examples, an unhandled rejection might be handled in the fu Certain unhandled rejections may in rare cases leave your application in a non-deterministic and unsafe state, whether it's internal application state (including memory leaks), external resources used by your application (say, file handles or database connections), or external state (say, consistency of data in a database). +As an example, the following server is not sending a response back to the client, causing a socket leak and a possible Denial of Service attack: + +```js +const http = require('http') +const server = http.createServer(handle) + +server.listen(3000) + +function handle (req, res) { + doStuff() + .then((body) => { + res.end(body) + }) +} + +function doStuff () { + if (Math.random() < 0.5) { + return Promise.reject(new Error('kaboom')) + } + + return Promise.resolve('hello world') +} +``` + ## Are you currently using Promises, async functions, a mix, or neither? (Multiple choice) @@ -173,7 +197,7 @@ Node.js code may wrap resource managing code in async/await: ```js // if this method wasn't async, node would crash by default -myEmitter.on('event', async () =>{ +myEmitter.on('event', async () => { // react to event databaseConnection.release(); // throws an error }); From 69d21d482af5596b48eec40ecf9069b442a55df5 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 20 May 2020 15:31:19 -0700 Subject: [PATCH 14/15] fixup! fixup! fixup! fixup! fixup! doc: add unhandled promise rejection survey --- surveys/promise-rejections/survey.md | 48 +++++++++++++++++++++------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 239d2d13..406a4552 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -45,6 +45,31 @@ async function () { Adding the `async` keyword to a function will turn any exceptions thrown inside that function (or any throw propagated from other functions called within it) into a rejection. The same happens when refactoring callback based code that throws into async functions / Promises. Below is an example of a callback based code refactored to Promises where exceptions become rejections: +```js +// Callback version +const { readFile } = require('fs'); + +function readJsonFile(file, cb) { + readFile(file, (err, data) => { + if (err) { + // If error while reading file, propagate the error via callback + return cb(err, null); + } + // Unexpected invalid JSON input, code will throw + cb(err, JSON.parse(data)); + }); +} +``` + +```js +const { readFile } = require('fs').promises; + +async function readJsonFile(file) { + // Promise is rejected if fails to read or if unexpected JSON input. + return JSON.parse(await readFile(file)); +} +``` + ## What is an unhandled rejection? There are two ways to handle rejections: by attaching a `.catch` handler to it, @@ -77,14 +102,13 @@ try { foo(); // 4. Unhandled, but execution continues -const rejected = foo(); // 5. Unhandled on current event loop turn - -// 4. ... but handled in a future turn of the event loop when the setTimeout -// callback is executed. +const rejected = foo(); // 5. Unhandled on current event loop turn, but handled + // in a future turn of the event loop when the + // setTimeout callback below is executed. setTimeout(() => rejected.catch(() => console.error("an error occured")), 100); ``` -As we can see in the examples, an unhandled rejection might be handled in the future, like example 4, but it might also stay unhandled forever (like example 5). +As we can see in the examples, an unhandled rejection might be handled in the future, like example 5, but it might also stay unhandled forever (like example 4). Certain unhandled rejections may in rare cases leave your application in a non-deterministic and unsafe state, whether it's internal application state (including memory leaks), external resources used by your application (say, file handles or database connections), or external state (say, consistency of data in a database). @@ -129,7 +153,7 @@ When consuming Promises, async functions or thenables, which of the options belo - [ ] `.catch()` - [ ] `try / catch` wrapping an `await` operation - - [ ] Leave the handling to my caller + - [ ] Leave the handling to someone else (my caller, global handler, etc.) - [ ] Not writing Promise-based code or using Promise-based libraries ## Do you know that Node.js has a global handler for unhandled rejections (`process.on('unhandledRejection')`)? If so, do you use it? @@ -150,9 +174,9 @@ When consuming Promises, async functions or thenables, which of the options belo ## Do you know that Node.js has a global handler for uncaught exception (`process.on('uncaughtException')`)? If so, do you use it? - - [ ] I use `process.on('unhandledRejection')` - - [ ] I don't use `process.on('unhandledRejection')` - - [ ] I didn't know `process.on('unhandledRejection')` existed + - [ ] I use `process.on('uncaughtException')` + - [ ] I don't use `process.on('uncaughtException')` + - [ ] I didn't know `process.on('uncaughtException')` existed Are you using Promises in any of the following kinds of applications? (check all that apply) @@ -198,8 +222,8 @@ Node.js code may wrap resource managing code in async/await: ```js // if this method wasn't async, node would crash by default myEmitter.on('event', async () => { - // react to event - databaseConnection.release(); // throws an error + await databaseConnection.getValue() // throws an error + databaseConnection.release(); // oops, release is never called }); ``` @@ -223,7 +247,7 @@ Consider the following modes: - `warn-with-error`: outputs a warning as soon as possible. Continues running after the warning is emitted. If the process exits and no status code was set, the process exits with an error code - `none`: do nothing -For all the modes, the action (raise an exceptiom output a warninig) will happen on `nextTick`. +For all the modes, the action (raise an exception output a warning) will happen on `nextTick`. Which one you think should be the default on Node.js? From 6848983dacac9e0f7415ccf29296788ec5b07123 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Wed, 20 May 2020 17:18:25 -0700 Subject: [PATCH 15/15] fixup! run eslint --- surveys/promise-rejections/survey.md | 34 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/surveys/promise-rejections/survey.md b/surveys/promise-rejections/survey.md index 406a4552..459de2b6 100644 --- a/surveys/promise-rejections/survey.md +++ b/surveys/promise-rejections/survey.md @@ -7,7 +7,7 @@ Today, Node.js handles unhandled rejections by emitting a deprecation warning to A Promise rejection indicates that something went wrong while executing a Promise or an async function. Rejections can occur in several situations: throwing inside an async function or a `Promise` executor/then/catch/finally callback, when calling the `reject` callback of an `executor`, or when calling `Promise.reject`. ```js -Promise.reject(new Error()) // This will result in a rejection +Promise.reject(new Error()); // This will result in a rejection new Promise((fulfill, reject) => { reject(new Error()); // This will result in a rejection @@ -15,29 +15,29 @@ new Promise((fulfill, reject) => { new Promise(() => { throw new Error(); // This will result in a rejection -}) +}); Promise.resolve().then(() => { throw new Error(); // This will result in a rejection -}) +}); -Promise.reject().then(() = {}, () => { +Promise.reject().then(() => {}, () => { throw new Error(); // This will result in a rejection -} +}); Promise.reject().catch(() => { throw new Error(); // This will result in a rejection -}) +}); Promise.resolve().finally(() => { throw new Error(); // This will result in a rejection }); -async function () { +async function foo() { throw new Error(); // This will result in a rejection } -async function () { +async function bar(a) { if (a === undefined) a(); // This will result in a rejection } @@ -97,7 +97,7 @@ foo() // 1. Unhandled at this point try { await foo(); } catch(e) { // 3. Handled - console.error("an error occured") + console.error("an error occured"); } foo(); // 4. Unhandled, but execution continues @@ -115,24 +115,24 @@ Certain unhandled rejections may in rare cases leave your application in a non-d As an example, the following server is not sending a response back to the client, causing a socket leak and a possible Denial of Service attack: ```js -const http = require('http') -const server = http.createServer(handle) +const http = require('http'); +const server = http.createServer(handle); -server.listen(3000) +server.listen(3000); function handle (req, res) { doStuff() .then((body) => { - res.end(body) - }) + res.end(body); + }); } function doStuff () { if (Math.random() < 0.5) { - return Promise.reject(new Error('kaboom')) + return Promise.reject(new Error('kaboom')); } - return Promise.resolve('hello world') + return Promise.resolve('hello world'); } ``` @@ -222,7 +222,7 @@ Node.js code may wrap resource managing code in async/await: ```js // if this method wasn't async, node would crash by default myEmitter.on('event', async () => { - await databaseConnection.getValue() // throws an error + await databaseConnection.getValue(); // throws an error databaseConnection.release(); // oops, release is never called }); ```