Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Vercel Queues support: docs and example app, Nitro types and a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
RihanArfan
force-pushed
the
feat/vercel-queues
branch
from
2029791 to
bbe5bf9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/presets/vercel/runtime/queue-handler.ts (1)
9-11: Remove the unnecessary type castas Request.The
event.reqobject is already aRequestinstance in the Vercel runtime context (as evidenced by cron-handler.ts usingevent.req.headers.get()andevent.req.waitUntilwithout casting). The type cast is redundant and should be removed for consistency with other handlers:return handler(event.req);instead of:
return handler(event.req as Request);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/vercel/runtime/queue-handler.ts` around lines 9 - 11, The return statement in the defineHandler wrapper unnecessarily casts event.req to Request; update the defineHandler callback so it calls handler(event.req) without the redundant "as Request" cast (locate the defineHandler(...) wrapper and the call to handler in queue-handler.ts and remove the type assertion on event.req).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/2.deploy/20.providers/vercel.md`:
- Around line 58-86: The docs incorrectly reference vercel.routeFunctionConfig;
update all occurrences (text and the example in the nitro config) to use
vercel.functionRules so the sample matches the implementation/types;
specifically replace the symbol vercel.routeFunctionConfig with
vercel.functionRules in the example config block and any explanatory text, and
keep the rest of the example keys (e.g., maxDuration, memory, regions,
experimentalTriggers) unchanged.
In `@examples/vercel-queues/package.json`:
- Around line 3-12: The package.json scripts reference Vite ("dev": "vite dev",
"build": "vite build") but Vite is not listed in devDependencies; update the
devDependencies section to include "vite" (e.g., add a "vite": "latest" or
specific version) so the "dev" and "build" scripts resolve reliably alongside
existing entries like "nitro".
In `@src/presets/vercel/utils.ts`:
- Around line 64-69: Typo fix: change "accesible" to "accessible" in the warning
message. Update the string passed to nitro.logger.warn that references
`experimentalTriggers` on `vercel.functions` so the sentence reads "Routes with
queue triggers are not accessible on the web." Keep the rest of the message
intact and ensure references to `vercel.functionRules` remain unchanged.
---
Nitpick comments:
In `@src/presets/vercel/runtime/queue-handler.ts`:
- Around line 9-11: The return statement in the defineHandler wrapper
unnecessarily casts event.req to Request; update the defineHandler callback so
it calls handler(event.req) without the redundant "as Request" cast (locate the
defineHandler(...) wrapper and the call to handler in queue-handler.ts and
remove the type assertion on event.req).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 51894fd6-4700-46c8-8dc2-c7adc93647c3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
docs/2.deploy/20.providers/vercel.mddocs/4.examples/vercel-queues.mdexamples/vercel-queues/README.mdexamples/vercel-queues/nitro.config.tsexamples/vercel-queues/package.jsonexamples/vercel-queues/plugins/queue.tsexamples/vercel-queues/routes/send.tsexamples/vercel-queues/tasks/notifications/send.tsexamples/vercel-queues/tsconfig.jsonexamples/vercel-queues/vite.config.tspackage.jsonsrc/presets/vercel/preset.tssrc/presets/vercel/runtime/queue-handler.tssrc/presets/vercel/types.tssrc/presets/vercel/utils.tstest/fixture/nitro.config.tstest/presets/vercel.test.ts
| import { defineHandler } from "nitro"; | ||
| import { useNitroHooks } from "nitro/app"; | ||
|
|
||
| const handler = handleCallback(async (message, metadata) => { |
There was a problem hiding this comment.
Checking pkg implementation, handleCallback itself returns a Promise, it will be at least a dangling promise! We should either move to each request or at least use defineLazyHander or avoid non handled promise
There was a problem hiding this comment.
From claude:
import { handleCallback } from "@vercel/queue";
import { defineHandler } from "nitro";
import { useNitroHooks } from "nitro/app";
import { createClient } from "@vercel/queue"; // or however the client is constructed
const client = createClient({ token: process.env.QUEUE_TOKEN });
export default defineHandler(async (event) => {
const request = event.req as Request;
// Parse the callback request body that Vercel sends
const body = await request.json(); // { queueName, consumerGroup, messageId, ... }
await handleCallback(
async (message, metadata) => {
await useNitroHooks().callHook("vercel:queue", { message, metadata });
},
body, // the parsed callback request
{ client } // required options
);
return new Response("OK", { status: 200 });
});There was a problem hiding this comment.
The @vercel/queue handleCallback returns a promise for a Response ((req) => Promise<Response>). Anything returned by Nitro gets passed up to it and then returned. Do we still need this?
We could move the handler inside the request handler.
A lazily-created default client auto-detects the region from VERCEL_REGION, falling back to iad1. https://vercel.com/docs/queues/sdk#top-level-exports
import { handleCallback } from "@vercel/queue";
import { defineHandler } from "nitro";
import { useNitroHooks } from "nitro/app";
export default defineHandler((event) => {
return handleCallback(async (message, metadata) => {
await useNitroHooks().callHook("vercel:queue", { message, metadata });
})(event.req as Request);
});There was a problem hiding this comment.
Please check actual implementation in dist. From what i see it was immediately calling.
|
|
||
| export default defineHandler((event) => { | ||
| return handleCallback(async (message, metadata) => { | ||
| await useNitroHooks().callHook("vercel:queue", { message, metadata }); |
There was a problem hiding this comment.
Can you confirm how error handling currently works? We might wrap it to console.error + use nito.captureError (so other plugins can see errors) and if workflows has special handling for errors rethrow or otherwise do not.
There was a problem hiding this comment.
if your queue handler throws an exception or returns an HTTP 5xx status code (including 500), Vercel Queues treats this as a failure and will automatically schedule the message for a retry. The platform also retries on other error status codes such as 408 (Request Timeout), 409 (Conflict), and 429 (Too Many Requests), but not for status codes outside this set. Retries are not triggered for successful (2xx) responses or for other error codes like 400 (Bad Request).
We now console error, nitro.captureError and rethrow
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/presets/vercel/utils.ts (1)
60-70:⚠️ Potential issue | 🔴 CriticalCritical: Missing closing brace causes build failure.
The
ifblock starting at line 60 is missing its closing}after the warning call. This causes all subsequent code (lines 70–189) to be incorrectly nested inside theifblock, which in turn makes theexport async function generateEdgeFunctionFilesat line 191 appear insidegenerateFunctionFiles—triggering the pipeline'sSyntaxError: 'import', and 'export' cannot be used outside of module code.🐛 Proposed fix
nitro.logger.warn( "`experimentalTriggers` on the base `vercel.functions` config applies to the catch-all function and is likely not what you want. " + "Routes with queue triggers are not accessible on the web. " + "Use `vercel.functionRules` to attach triggers to specific routes instead." ); + } const functionConfigPath = resolve(nitro.options.output.serverDir, ".vc-config.json");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/presets/vercel/utils.ts` around lines 60 - 70, The if block checking Array.isArray(baseFunctionConfig.experimentalTriggers) is missing its closing brace; insert a closing "}" immediately after the nitro.logger.warn(...) call so the warning is the only statement inside that if, restoring correct scope for subsequent declarations (e.g., functionConfigPath, generateFunctionFiles) and ensuring generateEdgeFunctionFiles remains a top-level export.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/presets/vercel/utils.ts`:
- Around line 60-70: The if block checking
Array.isArray(baseFunctionConfig.experimentalTriggers) is missing its closing
brace; insert a closing "}" immediately after the nitro.logger.warn(...) call so
the warning is the only statement inside that if, restoring correct scope for
subsequent declarations (e.g., functionConfigPath, generateFunctionFiles) and
ensuring generateEdgeFunctionFiles remains a top-level export.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 78ac9397-0dc9-4b2a-b447-bad2a2dc6766
📒 Files selected for processing (1)
src/presets/vercel/utils.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
RihanArfan
force-pushed
the
feat/vercel-queues
branch
from
db5a896 to
20f3f60
Compare
commit: |
2 participants
🔗 Linked issue
Related #1974
❓ Type of change
📚 Description
Support Vercel Queues triggers through Nitro hooks, and documents using it with Nitro Tasks.
📝 Checklist