Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rule) Warn for awaits without a try/catch wrapper #201

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
```
Expand Down Expand Up @@ -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**

Expand Down Expand Up @@ -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
Expand Down
106 changes: 106 additions & 0 deletions __tests__/wrap-await-with-try-catch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
'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){
errorHandler(ex)
}
}`,
`async function test() {
try {
throw Error
}
catch(ex0){
try {
await doSomething()
}
catch(ex1) {
errorHandler(ex1)
}
}
}`,
`async function test() {
try {
(async function innerFn() {
try {
await doSomething()
}
catch (ex) {
errorHandler(ex)
}
})()
}
catch (ex) {
errorHandler(ex)
}
}`,
`var foo = async () => {
try {
await fetch();
} catch (error) {
errorHandler(error);
}
}`
],
invalid: [
{
code: 'async function hi() { await doSomething() }',
errors: [{ message }]
},
{
code: `async function test() {
try {
await doSomething()
}
finally {
errorHandler("ok.")
}
}`,
errors: [{ message }]
},
{
code: `async function test() {
try {
throw Error
}
catch (ex) {
await doSomethingOther()
}
}`,
errors: [{ message }]
},
{
code: `async function test() {
try {
(async function innerFn() {
await doSomething()
})()
}
catch (ex) {
errorHandler(ex)
}
}`,
errors: [{ message }]
},
{
code: `var foo = async () => {
await fetch();
}`,
errors: [{ message }]
}
]
})
76 changes: 76 additions & 0 deletions docs/rules/wrap-await-with-try-catch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Wrap awaits with try/catch blocks (wrap-await-with-try-catch)

## 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.
6 changes: 4 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'
}
}
}
Expand Down
91 changes: 91 additions & 0 deletions rules/wrap-await-with-try-catch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Rule: wrap-await-with-try-catch
*/

'use strict'

const getDocsUrl = require('./lib/get-docs-url')

module.exports = {
meta: {
type: 'suggestion',
docs: {
url: getDocsUrl('wrap-await-with-try-catch')
}
},
create(context) {
function isAwaitHandled() {
const ancestors = context.getAncestors()
let 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 &&
ancestor.parent.type === 'TryStatement' &&
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
}

let lastItem = handledInOrder[handledInOrder.length - 1]

while (
handledInOrder.length > 0 &&
!(lastItem.name === 'try' && lastItem.node.handler)
) {
const 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,
message: '"await"s must be wrapped with a try/catch statement.'
})
}
}
}
}