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(linter): add oxc/no-async-endpoint-handlers #5364

Merged

Conversation

DonIsaac
Copy link
Contributor

Adds no-async-endpoint-handlers rules, which bans async functions used as endpoint handlers in Express applications. These do not get caught by Express' error handler, causing the server to crash with an unhandled process rejection error.

app.use(async (req, res) => {
  const foo = await api.getFoo(req.query) // server panics if this function rejects
  return res.json(foo)
})

I could not find this rule implemented in any ESLint plugin, but this is a problem I see quite often and I'm tired of dealing with it. I've added it to oxc for now, but we should consider adding an express or api plugin in the future.

@DonIsaac DonIsaac added C-enhancement Category - New feature or request A-linter Area - Linter labels Aug 31, 2024
Copy link

graphite-app bot commented Aug 31, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added A-ast Area - AST A-transformer Area - Transformer / Transpiler labels Aug 31, 2024
Copy link

codspeed-hq bot commented Aug 31, 2024

CodSpeed Performance Report

Merging #5364 will not alter performance

Comparing don/08-31-feat_linter_add_oxc/no-async-endpoint-handlers_ (f81e8a1) with main (add1465)

Summary

✅ 28 untouched benchmarks

@DonIsaac DonIsaac self-assigned this Aug 31, 2024
@DonIsaac DonIsaac force-pushed the don/08-31-feat_linter_add_oxc/no-async-endpoint-handlers_ branch from 16b1b4e to a341a83 Compare August 31, 2024 16:52
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @DonIsaac and the rest of your teammates on Graphite Graphite

@DonIsaac DonIsaac force-pushed the don/08-31-feat_linter_add_oxc/no-async-endpoint-handlers_ branch from a341a83 to 113eaf9 Compare August 31, 2024 16:54
@DonIsaac DonIsaac added the 0-merge Merge with Graphite Merge Queue label Aug 31, 2024
Copy link

graphite-app bot commented Aug 31, 2024

Merge activity

  • Aug 31, 12:56 PM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Aug 31, 12:59 PM EDT: DonIsaac added this pull request to the Graphite merge queue.
  • Aug 31, 1:03 PM EDT: DonIsaac merged this pull request with the Graphite merge queue.

Adds `no-async-endpoint-handlers` rules, which bans async functions used as endpoint handlers in Express applications. These do not get caught by Express' error handler, causing the server to crash with an unhandled process rejection error.

```js
app.use(async (req, res) => {
  const foo = await api.getFoo(req.query) // server panics if this function rejects
  return res.json(foo)
})
```

I could not find this rule implemented in any ESLint plugin, but this is a problem I see quite often and I'm tired of dealing with it. I've added it to `oxc` for now, but we should consider adding an `express` or `api` plugin in the future.
@DonIsaac DonIsaac force-pushed the don/08-31-feat_linter_add_oxc/no-async-endpoint-handlers_ branch from 113eaf9 to f81e8a1 Compare August 31, 2024 16:59
@graphite-app graphite-app bot merged commit f81e8a1 into main Aug 31, 2024
24 checks passed
@graphite-app graphite-app bot deleted the don/08-31-feat_linter_add_oxc/no-async-endpoint-handlers_ branch August 31, 2024 17:03
@oxc-bot oxc-bot mentioned this pull request Sep 2, 2024
Boshen added a commit that referenced this pull request Sep 2, 2024
## [0.9.2] - 2024-09-02

### Features

- f81e8a1 linter: Add `oxc/no-async-endpoint-handlers` (#5364)
(DonIsaac)
- b103737 linter: Improve no-accumulating-spread (#5302) (camc314)
- 9c22ce9 linter: Add hyperlinks to diagnostic messages (#5318)
(DonIsaac)
- 1967c67 linter/eslint: Implement no-new-func (#5360) (dalaoshu)
- b867e5f linter/eslint-plugin-promise: Implement catch-or-return
(#5121) (Jelle van der Waa)
- 8d781e7 linter/oxc: Differentiate between array/object in
`no-accumulating-spread` loop diagnostic (#5375) (camc314)
- db55444 linter/oxc: Add fixer for `double-comparisons` (#5378)
(camc314)
- e5c755a linter/promise: Add `spec-only` rule (#5124) (Jelle van der
Waa)
- 4c0861f linter/unicorn: Add fixer for `prefer-type-error` (#5311)
(camc314)
- 084c2d1 linter/vitest: Implement prefer-to-be-object (#5321)
(dalaoshu)

### Bug Fixes

- 11b93af linter/unicorn: Consistent-function-scoping false positive on
assignment expression (#5312) (Arian94)

### Performance

- f052a6d linter: `react/jsx_no_undef` faster check for unbound
references (#5349) (overlookmotel)
- 05636b7 linter: Avoid unnecessary work in `jsx_a11y/anchor_is_valid`
rule (#5341) (overlookmotel)

### Refactor

- afb038e linter: `react/jsx_no_undef` use loop instead of recursion
(#5347) (overlookmotel)
- fe62687 linter: Simplify skipping JSX elements in
`unicorn/consistent_function_scoping` (#5351) (overlookmotel)
- 381d9fe linter: Shorten code in `react/jsx_no_useless_fragment`
(#5350) (overlookmotel)
- 83b9a82 linter: Fix indentation in
`nextjs/no_script_component_in_head` rule (#5338) (overlookmotel)
- 89f0188 linter: Improve docs for `react/jsx_no_target_blank` rule
(#5342) (overlookmotel)
- 57050ab linter: Shorten code in
`jsx_a11y/aria_activedescendant_has_tabindex` rule (#5340)
(overlookmotel)
- ed31d67 linter/jest: Fix indentation in code comment (#5372) (camc314)
- 2499cb9 linter/oxc: Update rule docs for `erasing-op` (#5376)
(camc314)
- 69493d2 linter/oxc: Improve diagnostic for `no-accumulating-spread` in
loops (#5374) (camc314)
- 024b585 linter/oxc: Improve code comment for `no-accumulating-spread`
(#5373) (camc314)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
@oxc-bot oxc-bot mentioned this pull request Sep 3, 2024
Boshen added a commit that referenced this pull request Sep 3, 2024
## [0.26.0] - 2024-09-03

- 1aa49af ast: [**BREAKING**] Remove
`JSXMemberExpressionObject::Identifier` variant (#5358) (Dunqing)

- 01cc2ce semantic: [**BREAKING**] Add `ScopeTree:get_child_ids` API
behind a runtime flag (#5403) (Boshen)

- b1d0075 napi/transform: [**BREAKING**] Align output API `sourceText`
-> `code` with babel (#5398) (Boshen)

- 32f7300 ast: [**BREAKING**] Add `JSXElementName::IdentifierReference`
and `JSXMemberExpressionObject::IdentifierReference` (#5223) (Dunqing)

- 23e8456 traverse: [**BREAKING**] `TraverseCtx::ancestor` with level 0
= equivalent to `parent` (#5294) (overlookmotel)

- 582ce9e traverse: [**BREAKING**] `TraverseCtx::ancestor` return
`Ancestor::None` if out of bounds (#5286) (overlookmotel)

- 234a24c ast: [**BREAKING**] Merge `UsingDeclaration` into
`VariableDeclaration` (#5270) (Kevin Deng 三咲智子)

- c100826 semantic: [**BREAKING**] Always create a scope for `for`
statements (#5110) (overlookmotel)

- d304d6f semantic: [**BREAKING**] Always create a scope for
`CatchClause` (#5109) (overlookmotel)

### Features

- 180b1a1 ast: Add `Function::name()` (#5361) (DonIsaac)
- 5505749 ast: Add `accessibility` field to `AccessorProperty` (#5290)
(Dunqing)
- 49cd5db ast,parser: Add `definite` flag to `AccessorProperty` node
(#5182) (DonIsaac)
- c2fa725 ast,parser: Parse `TSTypeAnnotations` on `AccessorProperty`
(#5179) (DonIsaac)
- 292d162 codegen: Print missing fields for `AccessorProperty` (#5291)
(Dunqing)
- 72740b3 isolated_declaration: Support sourcemap option (#5170)
(dalaoshu)
- f81e8a1 linter: Add `oxc/no-async-endpoint-handlers` (#5364)
(DonIsaac)
- 9c22ce9 linter: Add hyperlinks to diagnostic messages (#5318)
(DonIsaac)
- d22bd20 module_lexer: Distinguish for types-only imports and exports
(#5184) (Kevin Deng 三咲智子)
- 7dfd51a parser: Report class properties that are both definite and
optional (#5181) (DonIsaac)
- a563968 parser: Report errors on optional accessor properties (#5180)
(DonIsaac)
- 46b641b regular_expression: Validate max quantifier value (#5218)
(leaysgur)
- be4642f semantic: Transform checker check child scope IDs (#5410)
(overlookmotel)
- 6e969f9 semantic: Add `ScopeTree::delete_root_unresolved_reference`
(#5305) (overlookmotel)
- 1b20ceb span: Add `CompactStr::to_compact_string` method (#5385)
(Boshen)
- 5a137f0 span/source-type: Add SourceType factory methods (#5242)
(DonIsaac)
- f5e05db span/source-type: Impl `Display` and `Error` for
`UnknownExtension` (#5240) (DonIsaac)
- d04857b transformer: Support `Targets::from_query` method (#5336)
(Dunqing)
- 3d4a64c transformer: Make `Targets` public (#5335) (Dunqing)
- 0eb7602 transformer: Support `TransformOptions::from_preset_env` API
(#5323) (Dunqing)
- 08dc0ad transformer: Add `object-spread` plugin (#3133) (magic-akari)
- 01c0c3e transformer: Add remaining options to transformer options
(#5169) (Boshen)
- 056c667 transformer/arrow-functions: The output that uses `this`
inside blocks doesn't match Babel (#5188) (Dunqing)
- 0abfc50 transformer/typescript: Support `rewrite_import_extensions`
option (#5399) (Dunqing)

### Bug Fixes

- 8ebc23f ast: Serialize `TSParenthesizedType` with camelCase (#5199)
(Kevin Deng 三咲智子)
- 5c4c001 codegen: Print `export @decorator declare abstract class Foo`
correctly (#5303) (Boshen)
- 7b1546b codegen: Do not print comments when `--minify` (Boshen)
- ff7fa98 diagnostics: Improve "file is too long to fit on the screen"
(#5120) (Boshen)
- 8a17807 parser: Treat JSX element tags starting with `_` or `$` as
`IdentifierReference`s (#5343) (overlookmotel)
- d4c06ef parser: Revert "check for `@flow` with recoverable errors as
well" (#5297) (overlookmotel)
- e1d8b92 parser: Check for `@flow` with recoverable errors as well
(Boshen)
- e6fd52e parser: Change unterminated regex error to be non-recoverable
(#5285) (Boshen)
- 1686920 parser: Span for invalid regex flags (#5225) (leaysgur)
- cffce11 regular_expression: Prevent panic on too large number (#5282)
(leaysgur)
- 293413f semantic: Implicit return `UpdateExpression` in
`ArrowFunctionExpression` does not as read reference (#5161) (Dunqing)
- a6bb3b1 span/source-type: Consider `.cjs` and `.cts` files as
`ModuleKind::Script` (#5239) (DonIsaac)
- 35f03db transformer: `ArrowfunctionExpression`'s expression is true
but has more than one body statement (#5366) (Dunqing)
- 8d6b05c transformer: Class property with typescript value should not
be removed (#5298) (Boshen)
- 47e69a8 transformer-optional-catch-binding: The `unused` binding is
not in the correct scope (#5066) (Dunqing)
- 94ff94c transformer/arrow-functions: Reaches `unreachable` when
`<this.foo>` is inside an arrow function (#5356) (Dunqing)
- f8bb022 transformer/arrow-functions: Remove
`SymbolFlags::ArrowFunction` (#5190) (Dunqing)
- d9ba5ad transformer/arrow-functions: Correct scope for `_this` (#5189)
(Dunqing)
- 3acb3f6 transformer/react: Mismatch output caused by incorrect
transformation ordering (#5255) (Dunqing)
- 5754c89 transformer/typescript: Remove accessibility from
`AccessorProperty` (#5292) (Dunqing)
- d594818 traverse: `insert_scope_below` update child scopes records
(#5409) (overlookmotel)
- 25d6e20 traverse: Add missing visitors to `ChildScopeCollector`
(#5118) (overlookmotel)

### Performance

- 292f217 ast: Optimize `JSXIdentifier::is_reference` (#5344)
(overlookmotel)
- 12a7607 codegen: Inline `Codegen::print_list` (#5221) (overlookmotel)
- fb847bd codegen: Slightly faster `print_list` (#5192) (Boshen)
- a1523c6 transformer: Remove an allocation from arrow functions
transform (#5412) (overlookmotel)

### Documentation

- 8334bd4 transformer: Add documentation for `Targets::get_targets`
(#5337) (Dunqing)
- d51a954 transformer: Add documentation for arrow-functions plugin
(#5186) (Dunqing)

### Refactor

- c2d8c9e ast: Reduce allocations in
`AstBuilder::move_assignment_target` (#5367) (overlookmotel)
- 946c867 ast: Box `TSThisParameter` (#5325) (overlookmotel)
- 960e1d5 ast: Rename `IdentifierReference::new_with_reference_id`
(#5157) (overlookmotel)
- f63b568 ast: Remove `#[non_exhaustive]` attr from `AstBuilder` (#5130)
(overlookmotel)
- d4c3778 codegen: Rename vars (#5222) (overlookmotel)
- 543cad6 codegen: Remove some pub APIs (Boshen)
- cd63336 diagnostic: Change how diagnostic codes are rendered (#5317)
(DonIsaac)
- d236554 parser: Move `JSXIdentifier` conversion code into parser
(#5345) (overlookmotel)
- bc59dd2 parser: Improve example for `byte_search!` macro usage (#5234)
(overlookmotel)
- a3ddfdd parser: Improve lexer pointer maths (#5233) (overlookmotel)
- 3ae94b8 semantic: Change `build_module_record` to accept &Path instead
of PathBuf (Boshen)
- 05d25e2 semantic: Combine add scope methods (#5262) (overlookmotel)
- fdedc0f semantic: Transform checker: rename `SemanticData` to
`Scoping` (#5261) (overlookmotel)
- 1086109 semantic: Transform checker do not output spans in errors
(#5260) (overlookmotel)
- af5713e semantic: Transform checker continue checks if missing IDs
(#5259) (overlookmotel)
- 943454f semantic: Update transform checker for no conditional scopes
(#5252) (overlookmotel)
- 892a7fa semantic: Replace `ref` syntax (#5253) (overlookmotel)
- cbb4725 semantic: Add comment to transform checker (#5250)
(overlookmotel)
- a17cf33 semantic: Remove `ScopeTree::child_ids` (#5232) (Boshen)
- d5a4940 semantic: Rewrite handling of label statement errors (#5138)
(Boshen)
- 94f60e7 span/source-type: Make `SourceType` factories `const` (#5241)
(DonIsaac)
- 0de844d transformer: Remove unnecessary code from JSX transform
(#5339) (overlookmotel)
- 5136f01 transformer: Remove unnecessary type annotation (#5131)
(overlookmotel)
- 260c9d2 transformer/es2015: Move all entry points to implementation of
Traverse trait (#5187) (Dunqing)
- 1645115 transformer/object-reset-spread: Make plugin initialization
unconditional (#5319) (Dunqing)
- d2666fe transformer/object-rest-spread: Move plugin-relates files to
`object_rest_spread` mod (#5320) (Dunqing)
- 7e2a7af transformer/react: Remove `CalculateSignatureKey`
implementation from refresh (#5289) (Dunqing)
- b43a394 traverse: Correct code comments (#5293) (overlookmotel)
- d71f0ed traverse: Inline all passthrough methods (#5279)
(overlookmotel)
- 188ce07 traverse: Improve safety via type system (#5277)
(overlookmotel)
- 0f4a8b3 traverse: Add debug asserts for safety invariants (#5272)
(overlookmotel)
- 341e42a traverse: Make `Ancestor` an owned type (#5269)
(overlookmotel)
- eba5033 traverse: Codegen `ChildScopeCollector` (#5119)
(overlookmotel)
- f771d7c traverse: Remove unnecessary imports (#5116) (overlookmotel)
- c6590ae traverse: Move generated files into separate folder (#5115)
(overlookmotel)
- fc2e9ad traverse: Remove support for `#[scope(if(...))]` attr (#5114)
(overlookmotel)
- 1ba11a3 traverse: Refactor `ChildScopeCollector` (#5112)
(overlookmotel)
- 40e2f6e traverse: Remove unnecessary branch in `ChildScopeCollector`
(#5111) (overlookmotel)
- b39c0d6 wasm: Add `source_type` for parser, replace class options with
plain object (#5217) (Kevin Deng 三咲智子)

### Testing

- 7009177 parser: Fix incorrect flow error test (Boshen)
- be7b8c6 semantic: Add `JSXIdentifierReference`-related tests (#5224)
(Dunqing)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-merge Merge with Graphite Merge Queue A-ast Area - AST A-linter Area - Linter A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants