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.

Sign up for GitHub

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

Jump to bottom

Add ESLint Rule to Prevent Redirect in Try-Catch Block #68108

Open
wants to merge 8 commits into
base: canary
Choose a base branch
from
+294 −37
Open

Add ESLint Rule to Prevent Redirect in Try-Catch Block #68108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6ab51f0
feat(eslint-plugin-next): Add 'no-redirect-in-try-catch' ESLint rule
piotrski Jul 10, 2024
edd35c7
docs(eslint): Add docs for no-redirect-in-try-catch rule
piotrski Jul 11, 2024
95eef7b
feat(eslint-plugin-next): Add 'no-redirect-in-try-catch' rule to reco…
piotrski Jul 11, 2024
e104f11
Merge branch 'canary' into eslint-no-redirect-in-try-catch
piotrski Aug 29, 2024
c056e39
test(eslint-plugin-next): update test cases for `no-redirect-in-try-c…
piotrski Aug 29, 2024
09a132d
feat(eslint-plugin-next): enforce `unstable_rethrow` usage in try-cat…
piotrski Aug 29, 2024
91c8508
feat(eslint-plugin-next): handle import aliases for `redirect` and `u…
piotrski Aug 29, 2024
6362a6f
feat(eslint-plugin-next): rename `no-redirect-in-try-catch` to `no-re…
piotrski Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions docs/02-app/01-building-your-application/07-configuring/02-eslint.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,29 +93,30 @@ Next.js provides an ESLint plugin, [`eslint-plugin-next`](https://www.npmjs.com/

<Check size={18} /> Enabled in the recommended configuration

| | Rule | Description |
| :-----------------: | ------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------- |
| <Check size={18} /> | [@next/next/google-font-display](/docs/messages/google-font-display) | Enforce font-display behavior with Google Fonts. |
| <Check size={18} /> | [@next/next/google-font-preconnect](/docs/messages/google-font-preconnect) | Ensure `preconnect` is used with Google Fonts. |
| <Check size={18} /> | [@next/next/inline-script-id](/docs/messages/inline-script-id) | Enforce `id` attribute on `next/script` components with inline content. |
| <Check size={18} /> | [@next/next/next-script-for-ga](/docs/messages/next-script-for-ga) | Prefer `next/script` component when using the inline script for Google Analytics. |
| <Check size={18} /> | [@next/next/no-assign-module-variable](/docs/messages/no-assign-module-variable) | Prevent assignment to the `module` variable. |
| <Check size={18} /> | [@next/next/no-async-client-component](/docs/messages/no-async-client-component) | Prevent client components from being async functions. |
| <Check size={18} /> | [@next/next/no-before-interactive-script-outside-document](/docs/messages/no-before-interactive-script-outside-document) | Prevent usage of `next/script`'s `beforeInteractive` strategy outside of `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-css-tags](/docs/messages/no-css-tags) | Prevent manual stylesheet tags. |
| <Check size={18} /> | [@next/next/no-document-import-in-page](/docs/messages/no-document-import-in-page) | Prevent importing `next/document` outside of `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-duplicate-head](/docs/messages/no-duplicate-head) | Prevent duplicate usage of `<Head>` in `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-head-element](/docs/messages/no-head-element) | Prevent usage of `<head>` element. |
| <Check size={18} /> | [@next/next/no-head-import-in-document](/docs/messages/no-head-import-in-document) | Prevent usage of `next/head` in `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-html-link-for-pages](/docs/messages/no-html-link-for-pages) | Prevent usage of `<a>` elements to navigate to internal Next.js pages. |
| <Check size={18} /> | [@next/next/no-img-element](/docs/messages/no-img-element) | Prevent usage of `<img>` element due to slower LCP and higher bandwidth. |
| <Check size={18} /> | [@next/next/no-page-custom-font](/docs/messages/no-page-custom-font) | Prevent page-only custom fonts. |
| <Check size={18} /> | [@next/next/no-script-component-in-head](/docs/messages/no-script-component-in-head) | Prevent usage of `next/script` in `next/head` component. |
| <Check size={18} /> | [@next/next/no-styled-jsx-in-document](/docs/messages/no-styled-jsx-in-document) | Prevent usage of `styled-jsx` in `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-sync-scripts](/docs/messages/no-sync-scripts) | Prevent synchronous scripts. |
| <Check size={18} /> | [@next/next/no-title-in-document-head](/docs/messages/no-title-in-document-head) | Prevent usage of `<title>` with `Head` component from `next/document`. |
| <Check size={18} /> | @next/next/no-typos | Prevent common typos in [Next.js's data fetching functions](/docs/pages/building-your-application/data-fetching) |
| <Check size={18} /> | [@next/next/no-unwanted-polyfillio](/docs/messages/no-unwanted-polyfillio) | Prevent duplicate polyfills from Polyfill.io. |
| | Rule | Description |
| :-----------------: | ------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| <Check size={18} /> | [@next/next/google-font-display](/docs/messages/google-font-display) | Enforce font-display behavior with Google Fonts. |
| <Check size={18} /> | [@next/next/google-font-preconnect](/docs/messages/google-font-preconnect) | Ensure `preconnect` is used with Google Fonts. |
| <Check size={18} /> | [@next/next/inline-script-id](/docs/messages/inline-script-id) | Enforce `id` attribute on `next/script` components with inline content. |
| <Check size={18} /> | [@next/next/next-script-for-ga](/docs/messages/next-script-for-ga) | Prefer `next/script` component when using the inline script for Google Analytics. |
| <Check size={18} /> | [@next/next/no-assign-module-variable](/docs/messages/no-assign-module-variable) | Prevent assignment to the `module` variable. |
| <Check size={18} /> | [@next/next/no-async-client-component](/docs/messages/no-async-client-component) | Prevent client components from being async functions. |
| <Check size={18} /> | [@next/next/no-before-interactive-script-outside-document](/docs/messages/no-before-interactive-script-outside-document) | Prevent usage of `next/script`'s `beforeInteractive` strategy outside of `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-css-tags](/docs/messages/no-css-tags) | Prevent manual stylesheet tags. |
| <Check size={18} /> | [@next/next/no-document-import-in-page](/docs/messages/no-document-import-in-page) | Prevent importing `next/document` outside of `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-duplicate-head](/docs/messages/no-duplicate-head) | Prevent duplicate usage of `<Head>` in `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-head-element](/docs/messages/no-head-element) | Prevent usage of `<head>` element. |
| <Check size={18} /> | [@next/next/no-head-import-in-document](/docs/messages/no-head-import-in-document) | Prevent usage of `next/head` in `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-html-link-for-pages](/docs/messages/no-html-link-for-pages) | Prevent usage of `<a>` elements to navigate to internal Next.js pages. |
| <Check size={18} /> | [@next/next/no-img-element](/docs/messages/no-img-element) | Prevent usage of `<img>` element due to slower LCP and higher bandwidth. |
| <Check size={18} /> | [@next/next/no-page-custom-font](/docs/messages/no-page-custom-font) | Prevent page-only custom fonts. |
| <Check size={18} /> | [@next/next/no-script-component-in-head](/docs/messages/no-script-component-in-head) | Prevent usage of `next/script` in `next/head` component. |
| <Check size={18} /> | [@next/next/no-styled-jsx-in-document](/docs/messages/no-styled-jsx-in-document) | Prevent usage of `styled-jsx` in `pages/_document.js`. |
| <Check size={18} /> | [@next/next/no-sync-scripts](/docs/messages/no-sync-scripts) | Prevent synchronous scripts. |
| <Check size={18} /> | [@next/next/no-title-in-document-head](/docs/messages/no-title-in-document-head) | Prevent usage of `<title>` with `Head` component from `next/document`. |
| <Check size={18} /> | @next/next/no-typos | Prevent common typos in [Next.js's data fetching functions](/docs/pages/building-your-application/data-fetching) |
| <Check size={18} /> | [@next/next/no-unwanted-polyfillio](/docs/messages/no-unwanted-polyfillio) | Prevent duplicate polyfills from Polyfill.io. |
| <Check size={18} /> | [@next/next/no-redirect-in-try-catch-without-rethrow](/docs/messages/no-redirect-in-try-catch-without-rethrow) | Ensure that when using `redirect` within a try-catch block, the catch block must start with a call to `unstable_rethrow` to ensure proper error propagation. |

If you already have ESLint configured in your application, we recommend extending from this plugin directly instead of including `eslint-config-next` unless a few conditions are met. Refer to the [Recommended Plugin Ruleset](#recommended-plugin-ruleset) to learn more.

Expand Down
45 changes: 45 additions & 0 deletions errors/no-redirect-in-try-catch-without-rethrow.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
title: No Redirect in Try-Catch Without Rethrow
---

> Ensure that when using `redirect` within a try-catch block, the catch block must start with a call to `unstable_rethrow` to ensure proper error propagation.

## Why This Error Occurred

You attempted to use the `redirect` function within a try-catch block without rethrowing the error using `unstable_rethrow`. When `redirect` is called, it throws a `NEXT_REDIRECT` error internally. If this error is caught without being rethrown, it prevents the redirect from executing as intended and suppresses the error handling.

## Possible Ways to Fix It

To ensure proper error handling and that the redirect can proceed as intended, the catch block should start with a call to `unstable_rethrow`. This ensures that Next.js's internal error handling is respected and that the redirect is properly executed.

If you need to handle other errors, you can still do so after the `unstable_rethrow` call.

## Example

### Incorrect Usage:

```javascript
try {
// some code that might throw an error
redirect('/some-path')
} catch (error) {
// handle error
}
```

### Correct Usage:

```javascript
try {
// some code that might throw an error
redirect('/some-path')
} catch (error) {
unstable_rethrow(error)
// handle other errors if necessary
}
```

## Useful Links

- [Next.js Redirect Documentation](/docs/app/building-your-application/routing/redirecting#redirect-function)
- [Next.js Error Handling](/docs/app/building-your-application/routing/error-handling)
1 change: 1 addition & 0 deletions packages/eslint-plugin-next/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"fast-glob": "3.3.1"
},
"devDependencies": {
"@types/estree": "1.0.5",
"eslint": "8.56.0"
},
"scripts": {
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin-next/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = {
'no-title-in-document-head': require('./rules/no-title-in-document-head'),
'no-typos': require('./rules/no-typos'),
'no-unwanted-polyfillio': require('./rules/no-unwanted-polyfillio'),
'no-redirect-in-try-catch-without-rethrow': require('./rules/no-redirect-in-try-catch-without-rethrow'),
},
configs: {
recommended: {
Expand Down Expand Up @@ -49,6 +50,7 @@ module.exports = {
'@next/next/no-duplicate-head': 'error',
'@next/next/no-head-import-in-document': 'error',
'@next/next/no-script-component-in-head': 'error',
'@next/next/no-redirect-in-try-catch-without-rethrow': 'error',
},
},
'core-web-vitals': {
Expand Down
84 changes: 84 additions & 0 deletions packages/eslint-plugin-next/src/rules/no-redirect-in-try-catch-without-rethrow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { defineRule } from '../utils/define-rule'
import type { Node, BlockStatement, ImportDeclaration } from 'estree'

const url =
'https://nextjs.org/docs/messages/no-redirect-in-try-catch-without-rethrow'

export = defineRule({
meta: {
docs: {
description:
'Ensure that when using `redirect` within a try-catch block, the catch block must start with a call to `unstable_rethrow` to ensures that errors are correctly propagated.',
recommended: true,
url,
},
type: 'problem',
schema: [],
},
create(context) {
let redirectName = 'redirect'
let rethrowName = 'unstable_rethrow'

function checkImport(node: ImportDeclaration) {
if (node.source.value === 'next/navigation') {
node.specifiers.forEach((specifier) => {
if (specifier.type === 'ImportSpecifier') {
if (specifier.imported.name === 'redirect') {
redirectName = specifier.local.name
} else if (specifier.imported.name === 'unstable_rethrow') {
rethrowName = specifier.local.name
}
}
})
}
}

function isRethrowFirstStatement(blockStatement: BlockStatement): boolean {
const firstStatement = blockStatement.body[0]
return (
firstStatement?.type === 'ExpressionStatement' &&
firstStatement.expression.type === 'CallExpression' &&
firstStatement.expression.callee.type === 'Identifier' &&
firstStatement.expression.callee.name === rethrowName
)
}

function containsRedirectCall(node: Node): boolean {
switch (node.type) {
case 'ExpressionStatement':
return (
node.expression.type === 'CallExpression' &&
node.expression.callee.type === 'Identifier' &&
node.expression.callee.name === redirectName
)
case 'BlockStatement':
return node.body.some(containsRedirectCall)
case 'IfStatement':
return (
containsRedirectCall(node.consequent) ||
(node.alternate ? containsRedirectCall(node.alternate) : false)
)
default:
return false
}
}

return {
ImportDeclaration: checkImport,
TryStatement(node) {
const tryBlock = node.block
const catchBlock = node.handler.body

if (
containsRedirectCall(tryBlock) &&
!isRethrowFirstStatement(catchBlock)
) {
context.report({
node: catchBlock,
message: `When using \`redirect\` in a try-catch block, ensure you include \`unstable_rethrow\` at the start of the catch block to properly handle Next.js errors. See: ${url}`,
})
}
},
}
},
})
Loading
Loading