feat: fastify#90
Merged
waltergalvao merged 5 commits intomainfrom Feb 23, 2026
Merged
Conversation
sweetr-dev
Bot
added
the
large
WalkthroughMigrates the API from Express to Fastify: replaces Express dependencies with Fastify equivalents, converts routers and middleware to Fastify plugins and request/reply semantics, introduces a Fastify app bootstrap, and updates error handling and Sentry integration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
apps/api/src/lib/sentry.ts (1)
52-52:Sentry.setupFastifyErrorHandleris valid — be aware of default error-capture noise
Sentry.setupFastifyErrorHandleris the official API for registering a Fastify error handler, and the re-export pattern mirrors what was done for Express, so the change itself is correct.However, a known behaviour is that
setupFastifyErrorHandlerreports all errors by default, including handled 4xx responses (validation failures, 404s, etc.), producing significant Sentry noise. TheshouldHandleErroroption — which lets you control which errors get captured — is only available from version 9.9.0+, while the project currently pins@sentry/nodeat 9.3.0. If noise becomes a problem before an upgrade, a thin wrapper aroundsetupFastifyErrorHandlerthat installs a customsetErrorHandleron the Fastify instance (filtering below 500) is the available workaround.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/lib/sentry.ts` at line 52, Sentry.setupFastifyErrorHandler is a valid re-export but with `@sentry/node` pinned to 9.3.0 you cannot use the shouldHandleError option (added in 9.9.0) so Sentry will capture handled 4xx noise; either upgrade the dependency to `@sentry/node` >= 9.9.0 and pass a shouldHandleError predicate to Sentry.setupFastifyErrorHandler, or keep the current version and replace the direct re-export by creating a thin wrapper that calls Sentry.setupFastifyErrorHandler but also installs a Fastify setErrorHandler on the server to filter out errors with statusCode < 500 before forwarding to Sentry (reference Sentry.setupFastifyErrorHandler and the Fastify setErrorHandler you will wrap).apps/api/src/bull-mq/bull-board.router.ts (1)
27-38: Therequest.url.startsWithguard may be redundant.Since this
onRequesthook is scoped to thebullBoardRouterplugin and the only routes registered here are the Bull Board routes (underenv.BULLBOARD_PATH), the URL prefix check on line 28 is effectively always true. It's not harmful, but it adds a misleading condition that suggests other routes might exist in this scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/bull-mq/bull-board.router.ts` around lines 27 - 38, The onRequest hook in fastify.addHook("onRequest", ...) contains a redundant guard checking request.url.startsWith(env.BULLBOARD_PATH) because this hook is already registered inside the bullBoardRouter plugin whose routes are limited to that path; remove the condition and its early return so the hook always performs authentication for requests handled by this router (keep the auth(request.raw) call, env.BULLBOARD_USERNAME/env.BULLBOARD_PASSWORD checks, and the reply.header/reply.code(401).send logic intact).apps/api/src/fastify.ts (1)
51-59: GraphQL Yoga context type cast.The
as nevercast on line 58 suppresses type checking for the server context. This works but hides potential type mismatches between what Yoga expects and what's provided. Consider defining a proper server context type for the Yoga instance to ensure type safety.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/fastify.ts` around lines 51 - 59, The handler currently silences type checking by casting the context to `never` when calling `yoga.handleNodeRequestAndResponse`; remove the `as never` and instead define a proper server context type (e.g. `ServerContext`) that matches what your Yoga instance expects, update the Yoga creation to use that generic (e.g. `createYoga<ServerContext>(...)` or the equivalent in your codebase), and pass an object typed as `ServerContext` into `yoga.handleNodeRequestAndResponse` in the `app.route` handler so `app.route`, `yoga.graphqlEndpoint`, and `yoga.handleNodeRequestAndResponse` all use the correct context type and the compiler can validate it.apps/api/src/index.ts (1)
32-37: Graceful shutdown can throw without being caught.If
closeAllQueueWorkers()orapp.close()throws, the error becomes an unhandled rejection sinceshutdownGracefullyis invoked from signal handlers with no.catch(). Consider wrapping in try/catch so the process still exits.Proposed fix
const shutdownGracefully = async (signal: string) => { console.log(`🔶 Received ${signal}, closing server..`); - await closeAllQueueWorkers(); - await app.close(); - process.exit(0); + try { + await closeAllQueueWorkers(); + await app.close(); + } catch (err) { + console.error("Error during graceful shutdown", err); + } + process.exit(0); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/index.ts` around lines 32 - 37, shutdownGracefully currently awaits closeAllQueueWorkers() and app.close() without error handling, which can create unhandled rejections when invoked from signal handlers; wrap the body of shutdownGracefully in a try/catch/finally: in try await closeAllQueueWorkers() and await app.close(); in catch log the error (include context and the caught error) using the existing logger or console.error; in finally ensure process.exit(0) on success or process.exit(1) on error so the process always exits. Reference shutdownGracefully, closeAllQueueWorkers, and app.close when applying the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/billing/stripe.router.ts`:
- Around line 70-73: Replace the non-idiomatic Fastify 5 redirect usage
`reply.code(303).redirect(session.url!)` with the Fastify 5 recommended form by
passing the status code as the second argument to redirect; update the call in
the handler that returns the Stripe checkout session (the code that currently
returns `reply.code(303).redirect(session.url!)`) to use
`reply.redirect(session.url!, 303)` so the redirect status is explicit and
follows Fastify 5 conventions.
In `@apps/api/src/app/github/middlewares/validate-webhook.middleware.ts`:
- Line 17: The early-return condition "if (!isLive && 0) return;" disables the
intended dev-skip logic; change this to "if (!isLive) return;" in the
validate-webhook middleware (the line currently containing isLive check) so
webhook validation is skipped in non-live environments as intended, and update
or remove any misleading inline comment accordingly.
In
`@apps/api/src/app/integrations/slack/middlewares/validate-webhook.middleware.ts`:
- Around line 17-18: The Slack middleware currently reads req.headers into
signature and timestamp without null-safety which can cause
Buffer.from(undefined) or NaN timestamp checks; update the code in
validate-webhook.middleware.ts to default the header reads (e.g., const
signature = req.headers["x-slack-signature"] as string || "" and const timestamp
= req.headers["x-slack-request-timestamp"] as string || ""), and then explicitly
validate presence/format (if signature === "" or Number(timestamp) is NaN or the
timestamp is too old, return a 400/unauthorized) before calling Buffer.from(...)
or performing Math.abs(time - Number(timestamp)) checks so the middleware never
throws on missing headers.
In `@apps/api/src/index.ts`:
- Line 13: Remove the stray debug console.log that prints the certificate path
(the call using console.log(resolve(__dirname, "../../../certs/tls.key")) in
apps/api/src/index.ts); either delete the console.log entirely or replace it
with a proper logger call gated by a debug flag (e.g., use your project's logger
at startup or check NODE_ENV/DEBUG before logging) so cert paths are not
unconditionally printed in production.
In `@apps/api/src/lib/fastify-helpers.ts`:
- Around line 28-30: The fastify error handler currently calls
Error.captureStackTrace(error) before captureException(error), which overwrites
the original throw-site stack sent to Sentry; change the order so
captureException(error) is called before Error.captureStackTrace(error), or
remove the Error.captureStackTrace call entirely, and keep the existing
logger.error("Fastify error handler", error) and captureException(error) calls
intact (refer to logger.error, Error.captureStackTrace, and captureException in
the handler to locate the code).
---
Nitpick comments:
In `@apps/api/src/bull-mq/bull-board.router.ts`:
- Around line 27-38: The onRequest hook in fastify.addHook("onRequest", ...)
contains a redundant guard checking request.url.startsWith(env.BULLBOARD_PATH)
because this hook is already registered inside the bullBoardRouter plugin whose
routes are limited to that path; remove the condition and its early return so
the hook always performs authentication for requests handled by this router
(keep the auth(request.raw) call, env.BULLBOARD_USERNAME/env.BULLBOARD_PASSWORD
checks, and the reply.header/reply.code(401).send logic intact).
In `@apps/api/src/fastify.ts`:
- Around line 51-59: The handler currently silences type checking by casting the
context to `never` when calling `yoga.handleNodeRequestAndResponse`; remove the
`as never` and instead define a proper server context type (e.g.
`ServerContext`) that matches what your Yoga instance expects, update the Yoga
creation to use that generic (e.g. `createYoga<ServerContext>(...)` or the
equivalent in your codebase), and pass an object typed as `ServerContext` into
`yoga.handleNodeRequestAndResponse` in the `app.route` handler so `app.route`,
`yoga.graphqlEndpoint`, and `yoga.handleNodeRequestAndResponse` all use the
correct context type and the compiler can validate it.
In `@apps/api/src/index.ts`:
- Around line 32-37: shutdownGracefully currently awaits closeAllQueueWorkers()
and app.close() without error handling, which can create unhandled rejections
when invoked from signal handlers; wrap the body of shutdownGracefully in a
try/catch/finally: in try await closeAllQueueWorkers() and await app.close(); in
catch log the error (include context and the caught error) using the existing
logger or console.error; in finally ensure process.exit(0) on success or
process.exit(1) on error so the process always exits. Reference
shutdownGracefully, closeAllQueueWorkers, and app.close when applying the
change.
In `@apps/api/src/lib/sentry.ts`:
- Line 52: Sentry.setupFastifyErrorHandler is a valid re-export but with
`@sentry/node` pinned to 9.3.0 you cannot use the shouldHandleError option (added
in 9.9.0) so Sentry will capture handled 4xx noise; either upgrade the
dependency to `@sentry/node` >= 9.9.0 and pass a shouldHandleError predicate to
Sentry.setupFastifyErrorHandler, or keep the current version and replace the
direct re-export by creating a thin wrapper that calls
Sentry.setupFastifyErrorHandler but also installs a Fastify setErrorHandler on
the server to filter out errors with statusCode < 500 before forwarding to
Sentry (reference Sentry.setupFastifyErrorHandler and the Fastify
setErrorHandler you will wrap).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/billing/stripe.router.ts`:
- Around line 39-40: The catch block passes the catch variable (typed as
unknown) directly to captureException, causing a TypeScript type error and
unsafe handling; update the catch handling in stripe.router.ts to normalize the
thrown value to an Error before calling captureException: check if error
instanceof Error and pass it directly, otherwise wrap it (e.g., new
Error(String(error))) or create a safe Error with contextual message, then call
captureException with that Error; ensure any subsequent code that reads
error.message or error.stack uses the normalized Error object.
In `@apps/api/src/bull-mq/bull-board.router.ts`:
- Around line 22-45: The auth hook is being added synchronously before the
rateLimit plugin loads so rate limiting never runs; fix it by registering the
rateLimit plugin first (await scope.register(rateLimit, ...)) and then
registering a nested child plugin that adds the auth hook (use
scope.register(async (inner) => { inner.addHook("onRequest", authHook) ... await
inner.register(serverAdapter.registerPlugin()) }, { prefix: env.BULLBOARD_PATH
})); reference the existing symbols auth, scope.addHook, rateLimit,
serverAdapter.registerPlugin, and env.BULLBOARD_PATH and ensure the auth hook
and serverAdapter registration occur inside the child scope so the rateLimit
hook executes before auth.
---
Duplicate comments:
In `@apps/api/src/app/billing/stripe.router.ts`:
- Line 72: The code uses a non-null assertion on Stripe.Checkout.Session.url
when calling reply.redirect(session.url!, 303), which could redirect to the
literal "null" if the SDK returns null; update the handler to guard the value:
check that session.url is a non-empty string before calling reply.redirect, and
if it's missing, log/record the error (using the existing logger) and return an
appropriate error response (e.g., reply.code(500).send or throw an HttpError)
instead of redirecting; reference session.url, reply.redirect, and
Stripe.Checkout.Session in your change.
In `@apps/api/src/lib/fastify-helpers.ts`:
- Around line 28-32: The current change ensures we only synthesize a stack when
the Error lacks one: retain the guard as written so logger.error("Fastify error
handler", error) runs, then if (!error.stack) Error.captureStackTrace(error) is
executed to preserve the original throw-site stack when present, and finally
call captureException(error); ensure these exact symbols (logger.error,
Error.captureStackTrace, captureException) remain in this order and the if
(!error.stack) guard is not removed or inverted.
Contributor
Additional Comments (1)
|
sweetr-dev
Bot
added
large
sweetrdev
approved these changes
Feb 23, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Greptile Summary
This PR migrates the API from Express to Fastify. The migration includes updating all routers to Fastify's plugin pattern, replacing Express middleware with Fastify equivalents, and updating dependencies.
Key changes:
FastifyPluginAsyncpatternpreHandlerhooksfastify-raw-bodyplugin for webhook signature verification (Stripe, GitHub, Slack)Issues found:
session.url!non-null assertion in stripe.router.ts could cause issues if Stripe returns null (already noted in previous threads)Confidence Score: 4/5
session.url!in the Stripe router which was already noted in previous threads. All webhook validations are properly implemented, rawBody configuration is correct, and the middleware migrations follow Fastify best practices.Important Files Changed
&& 0), properly migrated to Fastify middleware patternFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Fastify App] --> B[Plugins Registration] B --> C[CORS Plugin] B --> D[Raw Body Plugin] B --> E[Form Body Plugin] B --> F[Content Type Parser] A --> G[Router Registration] G --> H[Health Router] G --> I[GitHub Router] G --> J[Stripe Router] G --> K[Slack Router] G --> L[BullBoard Router] G --> M[Deployments Router] G --> N[GraphQL Yoga] I --> O[Webhook Validation Middleware] O --> P[Signature Verification] J --> Q[Raw Body Config] Q --> R[Stripe Webhook Handler] Q --> S[Checkout Handler] K --> T[Slack Webhook Validation] T --> U[Timestamp Check] T --> V[Signature Verification] L --> W[Scoped Plugin] W --> X[Rate Limiting] W --> Y[Basic Auth] W --> Z[BullBoard UI] A --> AA[Error Handlers] AA --> AB[Sentry Integration] AA --> AC[Custom Error Handler]Last reviewed commit: 0ac92b0