Add oxlint rule to catch thrown un-awaited redirect helpers (#4222)
## ✅ Checklist - [x] I have followed every step in the [contributing guide](https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md) - [x] The PR title follows the convention. - [x] I ran and tested the code works --- ## Testing - Verified the rule emits exactly five errors for un-awaited throws of the known async redirect helpers while ignoring awaited throws, returned promises, and synchronous `redirect(...)`. - Verified `--fix` inserts `await` in async functions and produces a clean second lint run. - Verified synchronous functions remain diagnostic-only so autofix cannot introduce invalid syntax. - Ran `pnpm run format`, `pnpm run lint`, `pnpm run typecheck --filter webapp`, and `git diff --check`. --- ## Changelog Adds an Oxlint rule that prevents async redirect helpers from being thrown without awaiting their `Response`. Existing violations are fixed, the autofix is limited to async functions, and the plugin uses an explicit ESM extension. --- ## Screenshots See the test-results comment for CLI evidence. 💯 Link to Devin session: https://app.devin.ai/sessions/e60ad7610773401da3d3040cf1252337 Requested by: @ericallam --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Eric Allam <eric@trigger.dev>
This commit is contained in:
+3
-1
@@ -1,6 +1,7 @@
|
||||
{
|
||||
"$schema": "./node_modules/oxlint/configuration_schema.json",
|
||||
"plugins": ["typescript", "import", "react"],
|
||||
"jsPlugins": ["./oxlint-plugins/no-thrown-unawaited-redirect.mjs"],
|
||||
"ignorePatterns": [
|
||||
"**/dist/**",
|
||||
"**/build/**",
|
||||
@@ -31,6 +32,7 @@
|
||||
"import/no-duplicates": "error",
|
||||
"import/namespace": "off",
|
||||
"react-hooks/exhaustive-deps": "off",
|
||||
"react-hooks/rules-of-hooks": "off"
|
||||
"react-hooks/rules-of-hooks": "off",
|
||||
"trigger/no-thrown-unawaited-redirect": "error"
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -134,7 +134,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
|
||||
);
|
||||
|
||||
if (!project) {
|
||||
throw redirectWithErrorMessage(redirectPath, request, "Project not found");
|
||||
throw await redirectWithErrorMessage(redirectPath, request, "Project not found");
|
||||
}
|
||||
|
||||
const currentPlan = await getCurrentPlan(project.organizationId);
|
||||
|
||||
+1
-1
@@ -155,7 +155,7 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
|
||||
);
|
||||
|
||||
if (!project) {
|
||||
throw redirectWithErrorMessage(redirectPath, request, "Project not found");
|
||||
throw await redirectWithErrorMessage(redirectPath, request, "Project not found");
|
||||
}
|
||||
|
||||
const formData = await request.formData();
|
||||
|
||||
+2
-2
@@ -103,14 +103,14 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
|
||||
);
|
||||
|
||||
if (!project) {
|
||||
throw redirectWithErrorMessage(redirectPath, request, "Project not found");
|
||||
throw await redirectWithErrorMessage(redirectPath, request, "Project not found");
|
||||
}
|
||||
|
||||
const formData = await request.formData();
|
||||
const parsedFormData = FormSchema.safeParse(Object.fromEntries(formData));
|
||||
|
||||
if (!parsedFormData.success) {
|
||||
throw redirectWithErrorMessage(redirectPath, request, "No region specified");
|
||||
throw await redirectWithErrorMessage(redirectPath, request, "No region specified");
|
||||
}
|
||||
|
||||
const service = new SetDefaultRegionService();
|
||||
|
||||
@@ -80,7 +80,7 @@ export const action = dashboardAction(
|
||||
});
|
||||
|
||||
if (!organization) {
|
||||
throw redirectWithErrorMessage(form.callerPath, request, "Organization not found");
|
||||
throw await redirectWithErrorMessage(form.callerPath, request, "Organization not found");
|
||||
}
|
||||
|
||||
let payload: SetPlanBody;
|
||||
@@ -139,7 +139,7 @@ export const action = dashboardAction(
|
||||
}
|
||||
case "paid": {
|
||||
if (form.planCode === undefined) {
|
||||
throw redirectWithErrorMessage(form.callerPath, request, "Not a valid plan");
|
||||
throw await redirectWithErrorMessage(form.callerPath, request, "Not a valid plan");
|
||||
}
|
||||
payload = {
|
||||
type: "paid" as const,
|
||||
|
||||
@@ -69,7 +69,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
|
||||
|
||||
if (!params.success) {
|
||||
logger.error("Invalid params for Vercel onboarding", { error: params.error });
|
||||
throw redirectWithErrorMessage(
|
||||
throw await redirectWithErrorMessage(
|
||||
"/",
|
||||
request,
|
||||
"Invalid installation parameters. Please try again from Vercel."
|
||||
@@ -89,7 +89,7 @@ export async function loader({ request }: LoaderFunctionArgs) {
|
||||
|
||||
if (!params.data.code) {
|
||||
logger.error("Missing code parameter for Vercel onboarding");
|
||||
throw redirectWithErrorMessage(
|
||||
throw await redirectWithErrorMessage(
|
||||
"/",
|
||||
request,
|
||||
"Invalid installation parameters. Please try again from Vercel."
|
||||
@@ -151,7 +151,11 @@ export async function loader({ request }: LoaderFunctionArgs) {
|
||||
organizationId: params.data.organizationId,
|
||||
userId,
|
||||
});
|
||||
throw redirectWithErrorMessage("/", request, "Organization not found. Please try again.");
|
||||
throw await redirectWithErrorMessage(
|
||||
"/",
|
||||
request,
|
||||
"Organization not found. Please try again."
|
||||
);
|
||||
}
|
||||
|
||||
return typedjson({
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
/**
|
||||
* oxlint custom rule: no-thrown-unawaited-redirect
|
||||
*
|
||||
* Catches `throw someRedirectHelper(...)` where the helper is an *async* function
|
||||
* that returns a Promise<Response> (e.g. `redirectWithErrorMessage`). Throwing the
|
||||
* un-awaited call throws a *pending Promise* instead of a Response, so Remix renders
|
||||
* the route's error boundary instead of performing the redirect.
|
||||
*
|
||||
* Correct forms are:
|
||||
* - `throw await redirectWithErrorMessage(...)`
|
||||
* - `return redirectWithErrorMessage(...)`
|
||||
*
|
||||
* Note: the plain synchronous `redirect(...)` from `remix-typedjson` returns a
|
||||
* `Response` directly, so `throw redirect(...)` is the intended Remix control-flow
|
||||
* pattern and is intentionally NOT flagged.
|
||||
*/
|
||||
|
||||
// Async redirect helpers that return a Promise. Extend this list as new async
|
||||
// redirect helpers are added.
|
||||
const ASYNC_REDIRECT_HELPERS = new Set([
|
||||
"redirectWithSuccessMessage",
|
||||
"redirectWithErrorMessage",
|
||||
"redirectBackWithErrorMessage",
|
||||
"redirectBackWithSuccessMessage",
|
||||
"redirectWithImpersonation",
|
||||
]);
|
||||
|
||||
const FUNCTION_TYPES = new Set([
|
||||
"ArrowFunctionExpression",
|
||||
"FunctionDeclaration",
|
||||
"FunctionExpression",
|
||||
]);
|
||||
|
||||
function isInsideAsyncFunction(node, sourceCode) {
|
||||
const ancestors = sourceCode.getAncestors(node);
|
||||
|
||||
for (let index = ancestors.length - 1; index >= 0; index--) {
|
||||
const ancestor = ancestors[index];
|
||||
|
||||
if (FUNCTION_TYPES.has(ancestor.type)) {
|
||||
return ancestor.async;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/** @type {import("eslint").Rule.RuleModule} */
|
||||
const noThrownUnawaitedRedirect = {
|
||||
meta: {
|
||||
type: "problem",
|
||||
docs: {
|
||||
description:
|
||||
"Disallow throwing an un-awaited async redirect helper (throws a pending Promise instead of a Response).",
|
||||
},
|
||||
fixable: "code",
|
||||
messages: {
|
||||
unawaited:
|
||||
'Throwing an un-awaited "{{name}}()" throws a pending Promise (Remix renders the error boundary instead of redirecting). Use "throw await {{name}}()" or "return {{name}}()".',
|
||||
},
|
||||
schema: [],
|
||||
},
|
||||
create(context) {
|
||||
return {
|
||||
ThrowStatement(node) {
|
||||
const argument = node.argument;
|
||||
|
||||
// Already awaited (`throw await helper()`) -> fine.
|
||||
if (!argument || argument.type === "AwaitExpression") {
|
||||
return;
|
||||
}
|
||||
|
||||
// Only care about direct calls: `throw helper(...)`.
|
||||
if (argument.type !== "CallExpression") {
|
||||
return;
|
||||
}
|
||||
|
||||
const callee = argument.callee;
|
||||
if (callee.type !== "Identifier" || !ASYNC_REDIRECT_HELPERS.has(callee.name)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const canAutofix = isInsideAsyncFunction(node, context.sourceCode);
|
||||
|
||||
context.report({
|
||||
node: argument,
|
||||
messageId: "unawaited",
|
||||
data: { name: callee.name },
|
||||
fix: canAutofix ? (fixer) => fixer.insertTextBefore(argument, "await ") : undefined,
|
||||
});
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
/** @type {import("eslint").ESLint.Plugin} */
|
||||
const plugin = {
|
||||
meta: {
|
||||
name: "trigger",
|
||||
},
|
||||
rules: {
|
||||
"no-thrown-unawaited-redirect": noThrownUnawaitedRedirect,
|
||||
},
|
||||
};
|
||||
|
||||
export default plugin;
|
||||
Reference in New Issue
Block a user