From f99f7e9e6121c502c28b2f894f814c2858e87530 Mon Sep 17 00:00:00 2001 From: Vedat Can Keklik Date: Thu, 16 Jul 2020 19:38:02 +0300 Subject: [PATCH 1/6] Initial commit for wrap-await-with-try-catch rule --- README.md | 5 +- docs/rules/wrap-await-with-try-catch.md | 79 +++++++++++++++++++++++++ index.js | 6 +- rules/wrap-await-with-try-catch.js | 64 ++++++++++++++++++++ 4 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 docs/rules/wrap-await-with-try-catch.md create mode 100644 rules/wrap-await-with-try-catch.js diff --git a/README.md b/README.md index 0ef515aa..386f13db 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,8 @@ Then configure the rules you want to use under the rules section. "promise/avoid-new": "warn", "promise/no-new-statics": "error", "promise/no-return-in-finally": "warn", - "promise/valid-params": "warn" + "promise/valid-params": "warn", + "promise/wrap-await-with-try-catch": "warn" } } ``` @@ -92,6 +93,7 @@ or start with the recommended rule set: | [`valid-params`][valid-params] | Ensures the proper number of arguments are passed to Promise functions | :warning: | | | [`prefer-await-to-then`][prefer-await-to-then] | Prefer `await` to `then()` for reading Promise values | :seven: | | | [`prefer-await-to-callbacks`][prefer-await-to-callbacks] | Prefer async/await to the callback pattern | :seven: | | +| [`wrap-await-with-try-catch`][wrap-await-with-try-catch] | Ensures `await` expressions are wrapped with a try/catch | :warning: | | **Key** @@ -126,6 +128,7 @@ or start with the recommended rule set: [valid-params]: docs/rules/valid-params.md [prefer-await-to-then]: docs/rules/prefer-await-to-then.md [prefer-await-to-callbacks]: docs/rules/prefer-await-to-callbacks.md +[wrap-await-with-try-catch]: docs/rules/wrap-await-with-try-catch.md [nodeify]: https://www.npmjs.com/package/nodeify [pify]: https://www.npmjs.com/package/pify [@macklinu]: https://github.com/macklinu diff --git a/docs/rules/wrap-await-with-try-catch.md b/docs/rules/wrap-await-with-try-catch.md new file mode 100644 index 00000000..226759d8 --- /dev/null +++ b/docs/rules/wrap-await-with-try-catch.md @@ -0,0 +1,79 @@ +# Wrap awaits with try/catch blocks (wrap-await-with-try-catch) + +Calling a Promise static method with `new` is invalid, resulting in a +`TypeError` at runtime. + +## Rule Details + +This rule is aimed at flagging awaits where they are not checked for possible rejections. + +Examples for **incorrect** code for this rule: + +```js +await doSomething() +``` + +```js +try { + ... +} +catch (ex) { + await doSomething(); +} +``` + +```js +try { + (async function someFn(){ + await doSomething(); + })(); +} +catch (ex) { + //... +} +``` + +Examples for **correct** code for this rule: + +```js +try { + await doSomething(); +} +catch (ex) { + //... +} +``` + +```js +try { + //... +} +catch (ex0) { + try { + await doSomething(); + } + catch (ex1) { + //... + } +} +``` + +```js +try { + (async function someFn(){ + try { + await doSomething(); + } + catch (exInner) { + //... + } + })(); +} +catch (ex) { + //... +} +``` + +## When Not To Use It + +If you handle awaits with a different error checking mechanism, you can disable this rule. diff --git a/index.js b/index.js index 6be65b45..d2674fb3 100644 --- a/index.js +++ b/index.js @@ -15,7 +15,8 @@ module.exports = { 'avoid-new': require('./rules/avoid-new'), 'no-new-statics': require('./rules/no-new-statics'), 'no-return-in-finally': require('./rules/no-return-in-finally'), - 'valid-params': require('./rules/valid-params') + 'valid-params': require('./rules/valid-params'), + 'wrap-await-with-try-catch': require('./rules/wrap-await-with-try-catch') }, rulesConfig: { 'param-names': 1, @@ -39,7 +40,8 @@ module.exports = { 'promise/avoid-new': 'off', 'promise/no-new-statics': 'error', 'promise/no-return-in-finally': 'warn', - 'promise/valid-params': 'warn' + 'promise/valid-params': 'warn', + 'promise/wrap-await-with-try-catch': 'warn' } } } diff --git a/rules/wrap-await-with-try-catch.js b/rules/wrap-await-with-try-catch.js new file mode 100644 index 00000000..4ef602e4 --- /dev/null +++ b/rules/wrap-await-with-try-catch.js @@ -0,0 +1,64 @@ +/** + * Rule: wrap-await-with-try-catch + */ + +'use strict' + +module.exports = { + rules: { + "wrap-await-with-try-catch": { + create: function(context) { + function isAwaitHandled() { + var ancestors = context.getAncestors(); + var handledInOrder = []; + + ancestors.forEach((ancestor) => { + if (ancestor.type === "TryStatement") { + handledInOrder.push({name: "try", node: ancestor, relatedTry: ancestor}); + } + else if (ancestor.type === "CatchClause") { + handledInOrder.push({name: "catch", node: ancestor, relatedTry: ancestor.parent}); + } + else if (ancestor.type === "BlockStatement" && ancestor.parent.finalizer === ancestor) { + handledInOrder.push({name: "finally", node: ancestor, relatedTry: ancestor.parent}); + } + else if (ancestor.type === "FunctionExpression" || ancestor.type === "FunctionDeclaration") { + // clear the current parents, we are in a new function + handledInOrder = []; + } + }); + + if (handledInOrder.length === 0) { + return false; + } + + var lastItem = handledInOrder[handledInOrder.length - 1]; + + while (handledInOrder.length > 0 && !(lastItem.name === "try" && lastItem.node.handler)) { + var tryToBeDeleted = lastItem.relatedTry; + + while (handledInOrder.length > 0 && lastItem.relatedTry == tryToBeDeleted) { + handledInOrder.pop(); + lastItem = handledInOrder[handledInOrder.length - 1]; + } + } + + return handledInOrder.length > 0; + } + + return { + AwaitExpression(node) { + if (isAwaitHandled()) { + return; + } + + context.report({ + node: node, + message: '"await"s must be wrapped with a try/catch statement.' + }); + } + }; + } + } + } +}; From 865761e7f4d39aff5cd925ab522e567ac5b84a9e Mon Sep 17 00:00:00 2001 From: Vedat Can Keklik Date: Thu, 16 Jul 2020 21:07:05 +0300 Subject: [PATCH 2/6] Refactor and eslint --- rules/wrap-await-with-try-catch.js | 121 +++++++++++++++++------------ 1 file changed, 73 insertions(+), 48 deletions(-) diff --git a/rules/wrap-await-with-try-catch.js b/rules/wrap-await-with-try-catch.js index 4ef602e4..2a37eb60 100644 --- a/rules/wrap-await-with-try-catch.js +++ b/rules/wrap-await-with-try-catch.js @@ -4,61 +4,86 @@ 'use strict' -module.exports = { - rules: { - "wrap-await-with-try-catch": { - create: function(context) { - function isAwaitHandled() { - var ancestors = context.getAncestors(); - var handledInOrder = []; +const getDocsUrl = require('./lib/get-docs-url') - ancestors.forEach((ancestor) => { - if (ancestor.type === "TryStatement") { - handledInOrder.push({name: "try", node: ancestor, relatedTry: ancestor}); - } - else if (ancestor.type === "CatchClause") { - handledInOrder.push({name: "catch", node: ancestor, relatedTry: ancestor.parent}); - } - else if (ancestor.type === "BlockStatement" && ancestor.parent.finalizer === ancestor) { - handledInOrder.push({name: "finally", node: ancestor, relatedTry: ancestor.parent}); - } - else if (ancestor.type === "FunctionExpression" || ancestor.type === "FunctionDeclaration") { - // clear the current parents, we are in a new function - handledInOrder = []; - } - }); +module.exports = { + meta: { + type: 'suggestion', + docs: { + url: getDocsUrl('wrap-await-with-try-catch') + } + }, + create(context) { + function isAwaitHandled() { + const ancestors = context.getAncestors() + let handledInOrder = [] - if (handledInOrder.length === 0) { - return false; - } + ancestors.forEach(ancestor => { + if (ancestor.type === 'TryStatement') { + handledInOrder.push({ + name: 'try', + node: ancestor, + relatedTry: ancestor + }) + } else if (ancestor.type === 'CatchClause') { + handledInOrder.push({ + name: 'catch', + node: ancestor, + relatedTry: ancestor.parent + }) + } else if ( + ancestor.type === 'BlockStatement' && + ancestor.parent.finalizer === ancestor + ) { + handledInOrder.push({ + name: 'finally', + node: ancestor, + relatedTry: ancestor.parent + }) + } else if ( + ancestor.type === 'FunctionExpression' || + ancestor.type === 'FunctionDeclaration' + ) { + // clear the current parents, we are in a new function + handledInOrder = [] + } + }) - var lastItem = handledInOrder[handledInOrder.length - 1]; + if (handledInOrder.length === 0) { + return false + } - while (handledInOrder.length > 0 && !(lastItem.name === "try" && lastItem.node.handler)) { - var tryToBeDeleted = lastItem.relatedTry; + let lastItem = handledInOrder[handledInOrder.length - 1] - while (handledInOrder.length > 0 && lastItem.relatedTry == tryToBeDeleted) { - handledInOrder.pop(); - lastItem = handledInOrder[handledInOrder.length - 1]; - } - } + while ( + handledInOrder.length > 0 && + !(lastItem.name === 'try' && lastItem.node.handler) + ) { + const tryToBeDeleted = lastItem.relatedTry - return handledInOrder.length > 0; - } + while ( + handledInOrder.length > 0 && + lastItem.relatedTry == tryToBeDeleted + ) { + handledInOrder.pop() + lastItem = handledInOrder[handledInOrder.length - 1] + } + } - return { - AwaitExpression(node) { - if (isAwaitHandled()) { - return; - } + return handledInOrder.length > 0 + } - context.report({ - node: node, - message: '"await"s must be wrapped with a try/catch statement.' - }); - } - }; - } + return { + AwaitExpression(node) { + if (isAwaitHandled()) { + return } + + context.report({ + node, + message: '"await"s must be wrapped with a try/catch statement.' + }) + } } -}; + } +} From 87dd714ade820e7e1fe50026fbcd59bf45e74498 Mon Sep 17 00:00:00 2001 From: Vedat Can Keklik Date: Thu, 16 Jul 2020 21:31:03 +0300 Subject: [PATCH 3/6] Doc and some test cases --- __tests__/wrap-await-with-try-catch.js | 65 +++++++++++++++++++++++++ docs/rules/wrap-await-with-try-catch.md | 3 -- 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 __tests__/wrap-await-with-try-catch.js diff --git a/__tests__/wrap-await-with-try-catch.js b/__tests__/wrap-await-with-try-catch.js new file mode 100644 index 00000000..65b553c0 --- /dev/null +++ b/__tests__/wrap-await-with-try-catch.js @@ -0,0 +1,65 @@ +'use strict' + +const RuleTester = require('eslint').RuleTester +const rule = require('../rules/wrap-await-with-try-catch') +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 8 + } +}) + +const message = '"await"s must be wrapped with a try/catch statement.' + +ruleTester.run('wrap-await-with-try-catch', rule, { + valid: [ + `async function test() { + try { + await doSomething() + } + catch(ex){ + console.log(ex) + } + }`, + `async function test() { + try { + throw Error + } + catch(ex0){ + try { + await doSomething() + } + catch(ex1) { + console.log(ex1) + } + } + }` + ], + invalid: [ + { + code: 'async function hi() { await doSomething() }', + errors: [{ message }] + }, + { + code: `async function test() { + try { + await doSomething() + } + finally { + console.log("ok.") + } + }`, + errors: [{ message }] + }, + { + code: `async function test() { + try { + throw Error + } + catch (ex) { + await doSomethingOther() + } + }`, + errors: [{ message }] + } + ] +}) diff --git a/docs/rules/wrap-await-with-try-catch.md b/docs/rules/wrap-await-with-try-catch.md index 226759d8..f95364aa 100644 --- a/docs/rules/wrap-await-with-try-catch.md +++ b/docs/rules/wrap-await-with-try-catch.md @@ -1,8 +1,5 @@ # Wrap awaits with try/catch blocks (wrap-await-with-try-catch) -Calling a Promise static method with `new` is invalid, resulting in a -`TypeError` at runtime. - ## Rule Details This rule is aimed at flagging awaits where they are not checked for possible rejections. From ef412086eb327bce5c3e65a18a436cd1368c155b Mon Sep 17 00:00:00 2001 From: Vedat Can Keklik Date: Thu, 16 Jul 2020 21:40:41 +0300 Subject: [PATCH 4/6] More test cases --- __tests__/wrap-await-with-try-catch.js | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/__tests__/wrap-await-with-try-catch.js b/__tests__/wrap-await-with-try-catch.js index 65b553c0..3fe794b6 100644 --- a/__tests__/wrap-await-with-try-catch.js +++ b/__tests__/wrap-await-with-try-catch.js @@ -32,6 +32,21 @@ ruleTester.run('wrap-await-with-try-catch', rule, { console.log(ex1) } } + }`, + `async function test() { + try { + (async function innerFn() { + try { + await doSomething() + } + catch (ex) { + console.log(ex) + } + })() + } + catch (ex) { + console.log(ex) + } }` ], invalid: [ @@ -60,6 +75,19 @@ ruleTester.run('wrap-await-with-try-catch', rule, { } }`, errors: [{ message }] + }, + { + code: `async function test() { + try { + (async function innerFn() { + await doSomething() + })() + } + catch (ex) { + console.log(ex) + } + }`, + errors: [{ message }] } ] }) From 6cdc073a3c268a0677e451748744622d02b1bf7b Mon Sep 17 00:00:00 2001 From: Vedat Can Keklik Date: Thu, 16 Jul 2020 21:52:44 +0300 Subject: [PATCH 5/6] More tests --- __tests__/wrap-await-with-try-catch.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/__tests__/wrap-await-with-try-catch.js b/__tests__/wrap-await-with-try-catch.js index 3fe794b6..ba8938a8 100644 --- a/__tests__/wrap-await-with-try-catch.js +++ b/__tests__/wrap-await-with-try-catch.js @@ -17,7 +17,7 @@ ruleTester.run('wrap-await-with-try-catch', rule, { await doSomething() } catch(ex){ - console.log(ex) + errorHandler(ex) } }`, `async function test() { @@ -29,7 +29,7 @@ ruleTester.run('wrap-await-with-try-catch', rule, { await doSomething() } catch(ex1) { - console.log(ex1) + errorHandler(ex1) } } }`, @@ -40,12 +40,19 @@ ruleTester.run('wrap-await-with-try-catch', rule, { await doSomething() } catch (ex) { - console.log(ex) + errorHandler(ex) } })() } catch (ex) { - console.log(ex) + errorHandler(ex) + } + }`, + `var foo = async () => { + try { + await fetch(); + } catch (error) { + errorHandler(error); } }` ], @@ -60,7 +67,7 @@ ruleTester.run('wrap-await-with-try-catch', rule, { await doSomething() } finally { - console.log("ok.") + errorHandler("ok.") } }`, errors: [{ message }] @@ -84,10 +91,16 @@ ruleTester.run('wrap-await-with-try-catch', rule, { })() } catch (ex) { - console.log(ex) + errorHandler(ex) } }`, errors: [{ message }] + }, + { + code: `var foo = async () => { + await fetch(); + }`, + errors: [{ message }] } ] }) From ea8662c46c13e5d11c144febed7961aa70781668 Mon Sep 17 00:00:00 2001 From: Vedat Can Keklik Date: Fri, 17 Jul 2020 10:49:16 +0300 Subject: [PATCH 6/6] Defensive check for `finally` --- rules/wrap-await-with-try-catch.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rules/wrap-await-with-try-catch.js b/rules/wrap-await-with-try-catch.js index 2a37eb60..dd672c16 100644 --- a/rules/wrap-await-with-try-catch.js +++ b/rules/wrap-await-with-try-catch.js @@ -33,6 +33,8 @@ module.exports = { }) } else if ( ancestor.type === 'BlockStatement' && + ancestor.parent && + ancestor.parent.type === 'TryStatement' && ancestor.parent.finalizer === ancestor ) { handledInOrder.push({