From 384abae1ad9f90f1d5942c5029130e3390108ca1 Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Thu, 5 Feb 2026 16:11:23 +0000 Subject: [PATCH] feat(linter/oxc/no-async-endpoint-handlers): improve diagnostic message (#19001) fixes https://github.com/oxc-project/oxc/issues/18987 --- .../rules/oxc/no_async_endpoint_handlers.rs | 73 +++++++++++++++---- .../oxc_no_async_endpoint_handlers.snap | 60 +++++++++------ 2 files changed, 97 insertions(+), 36 deletions(-) diff --git a/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs b/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs index 63e116da53a9a..0fa3b2b3ea584 100644 --- a/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs +++ b/crates/oxc_linter/src/rules/oxc/no_async_endpoint_handlers.rs @@ -7,7 +7,7 @@ use oxc_ast::{ }; use oxc_diagnostics::{LabeledSpan, OxcDiagnostic}; use oxc_macros::declare_oxc_lint; -use oxc_span::{CompactStr, Span}; +use oxc_span::{Atom, CompactStr, Span}; use rustc_hash::FxHashSet; use schemars::JsonSchema; use serde_json::Value; @@ -35,6 +35,7 @@ pub fn no_async_handlers( function_span: Span, registered_span: Option, name: Option<&str>, + endpoint: Option<&str>, ) -> OxcDiagnostic { #[expect(clippy::cast_possible_truncation)] const ASYNC_LEN: u32 = "async".len() as u32; @@ -42,18 +43,24 @@ pub fn no_async_handlers( // Only cover "async" in "async function (req, res) {}" or "async (req, res) => {}" let async_span = Span::sized(function_span.start, ASYNC_LEN); + let registration_note = endpoint.map(|endpoint| format!(" for route `{endpoint}`")); + let registered_label = registration_note.as_deref().map_or_else( + || "and is registered here".to_string(), + |note| format!("and is registered here{note}"), + ); + let labels: &[LabeledSpan] = match (registered_span, name) { // handler is declared separately from registration // `async function foo(req, res) {}; app.get('/foo', foo);` (Some(span), Some(name)) => &[ async_span.label(format!("Async handler '{name}' is declared here")), - span.primary_label("and is registered here"), + span.primary_label(registered_label), ], // Shouldn't happen, since separate declaration/registration requires an // identifier to be bound (Some(span), None) => &[ async_span.label("Async handler is declared here"), - span.primary_label("and is registered here"), + span.primary_label(registered_label), ], // `app.get('/foo', async function foo(req, res) {});` (None, Some(name)) => &[async_span.label(format!("Async handler '{name}' is used here"))], @@ -62,9 +69,18 @@ pub fn no_async_handlers( (None, None) => &[async_span.label("Async handler is used here")], }; - OxcDiagnostic::warn("Express endpoint handlers should not be async.") + let warning = endpoint.map_or_else( + || "Express endpoint handler should not be `async`.".to_string(), + |endpoint| format!("Express endpoint handler for `{endpoint}` should not be `async`."), + ); + + OxcDiagnostic::warn(warning) .with_labels(labels.iter().cloned()) - .with_help("Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead.") + .with_help( + "Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`).\nExpress does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes.", + ).with_note( + "If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`." + ) } declare_oxc_lint!( @@ -180,26 +196,32 @@ impl Rule for NoAsyncEndpointHandlers { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { let kind = node.kind(); - let Some((_endpoint, args)) = utils::as_endpoint_registration(&kind) else { + let Some((endpoint, args)) = utils::as_endpoint_registration(&kind) else { return; }; for arg in args.iter().filter_map(Argument::as_expression).map(Expression::get_inner_expression) { - self.check_endpoint_arg(ctx, arg); + self.check_endpoint_arg(ctx, endpoint, arg); } } } impl NoAsyncEndpointHandlers { - fn check_endpoint_arg<'a>(&self, ctx: &LintContext<'a>, arg: &Expression<'a>) { + fn check_endpoint_arg<'a>( + &self, + ctx: &LintContext<'a>, + endpoint: Option>, + arg: &Expression<'a>, + ) { let mut visited = FxHashSet::default(); - self.check_endpoint_expr(ctx, None, None, arg, &mut visited); + self.check_endpoint_expr(ctx, endpoint, None, None, arg, &mut visited); } fn check_endpoint_expr<'a>( &self, ctx: &LintContext<'a>, + endpoint: Option>, id_name: Option<&str>, registered_at: Option, arg: &Expression<'a>, @@ -229,7 +251,9 @@ impl NoAsyncEndpointHandlers { let decl_node = ctx.nodes().get_node(decl_id); let registered_at = registered_at.or(Some(handler.span)); match decl_node.kind() { - AstKind::Function(f) => self.check_function(ctx, registered_at, id_name, f), + AstKind::Function(f) => { + self.check_function(ctx, endpoint, registered_at, id_name, f); + } AstKind::VariableDeclarator(decl) => { if let Some(init) = &decl.init { if let Expression::Identifier(id) = &init @@ -240,7 +264,14 @@ impl NoAsyncEndpointHandlers { { return; } - self.check_endpoint_expr(ctx, id_name, registered_at, init, visited); + self.check_endpoint_expr( + ctx, + endpoint, + id_name, + registered_at, + init, + visited, + ); } } _ => {} @@ -250,10 +281,10 @@ impl NoAsyncEndpointHandlers { match func { // `app.get('/', (async?) function (req, res) {}` Expression::FunctionExpression(f) => { - self.check_function(ctx, registered_at, id_name, f); + self.check_function(ctx, endpoint, registered_at, id_name, f); } Expression::ArrowFunctionExpression(f) => { - self.check_arrow(ctx, registered_at, id_name, f); + self.check_arrow(ctx, endpoint, registered_at, id_name, f); } _ => unreachable!(), } @@ -265,6 +296,7 @@ impl NoAsyncEndpointHandlers { fn check_function<'a>( &self, ctx: &LintContext<'a>, + endpoint: Option>, registered_at: Option, id_name: Option<&str>, f: &Function<'a>, @@ -278,12 +310,18 @@ impl NoAsyncEndpointHandlers { return; } - ctx.diagnostic(no_async_handlers(f.span, registered_at, name)); + ctx.diagnostic(no_async_handlers( + f.span, + registered_at, + name, + endpoint.map(|endpoint| endpoint.as_str()), + )); } fn check_arrow<'a>( &self, ctx: &LintContext<'a>, + endpoint: Option>, registered_at: Option, id_name: Option<&str>, f: &ArrowFunctionExpression<'a>, @@ -295,7 +333,12 @@ impl NoAsyncEndpointHandlers { return; } - ctx.diagnostic(no_async_handlers(f.span, registered_at, id_name)); + ctx.diagnostic(no_async_handlers( + f.span, + registered_at, + id_name, + endpoint.map(|endpoint| endpoint.as_str()), + )); } fn is_allowed_name(&self, name: &str) -> bool { diff --git a/crates/oxc_linter/src/snapshots/oxc_no_async_endpoint_handlers.snap b/crates/oxc_linter/src/snapshots/oxc_no_async_endpoint_handlers.snap index 34a007cdc7acb..44002e8fc7849 100644 --- a/crates/oxc_linter/src/snapshots/oxc_no_async_endpoint_handlers.snap +++ b/crates/oxc_linter/src/snapshots/oxc_no_async_endpoint_handlers.snap @@ -2,47 +2,57 @@ source: crates/oxc_linter/src/tester.rs --- - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:1:14] 1 │ app.get('/', async function (req, res) {}) · ──┬── · ╰── Async handler is used here ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:1:14] 1 │ app.get('/', async (req, res) => {}) · ──┬── · ╰── Async handler is used here ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:1:14] 1 │ app.get('/', async (req, res, next) => {}) · ──┬── · ╰── Async handler is used here ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:1:20] 1 │ weirdName.get('/', async (req, res) => {}) · ──┬── · ╰── Async handler is used here ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:1:20] 1 │ weirdName.get('/', async (req, res) => {}) · ──┬── · ╰── Async handler is used here ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:3:27] 1 │ 2 │ async function foo(req, res) {} @@ -50,12 +60,14 @@ source: crates/oxc_linter/src/tester.rs · ╰── Async handler 'foo' is declared here 3 │ app.post('/', foo) · ─┬─ - · ╰── and is registered here + · ╰── and is registered here for route `/` 4 │ ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:3:27] 1 │ 2 │ const foo = async (req, res) => {} @@ -63,12 +75,14 @@ source: crates/oxc_linter/src/tester.rs · ╰── Async handler is declared here 3 │ app.post('/', foo) · ─┬─ - · ╰── and is registered here + · ╰── and is registered here for route `/` 4 │ ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler should not be `async`. ╭─[no_async_endpoint_handlers.tsx:3:21] 1 │ 2 │ async function middleware(req, res, next) {} @@ -79,9 +93,11 @@ source: crates/oxc_linter/src/tester.rs · ╰── and is registered here 4 │ ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`. - ⚠ oxc(no-async-endpoint-handlers): Express endpoint handlers should not be async. + ⚠ oxc(no-async-endpoint-handlers): Express endpoint handler for `/` should not be `async`. ╭─[no_async_endpoint_handlers.tsx:4:27] 1 │ 2 │ async function foo(req, res) {} @@ -90,7 +106,9 @@ source: crates/oxc_linter/src/tester.rs 3 │ const bar = foo; 4 │ app.post('/', bar) · ─┬─ - · ╰── and is registered here + · ╰── and is registered here for route `/` 5 │ ╰──── - help: Express <= 4.x does not handle Promise rejections. Use `new Promise((resolve, reject) => { ... }).catch(next)` instead. + help: Wrap the async handler and forward errors to `next()` (e.g. `(req, res, next) => Promise.resolve(handler(req, res, next)).catch(next)`). + Express does not automatically handle rejected promises from async handlers, which results in unhandled promise rejections and server crashes. + note: If you're on Express 5, disable this rule. To allow specific functions, add their names to `allowedNames`.