Fixed /gift returning HTTP 404#27912
Conversation
- gift redemption links live at /gift/<token>, but a bare /gift or /gift/ (e.g. a site navigation link) hit the gift-preview sub-app and were terminated with a 404 by errorHandler.pageNotFound - the sub-app should only handle tokened requests; tokenless ones need to fall through to Ghost's regular frontend routing so a custom page (or routes.yaml entry) at /gift/ can render normally, or Ghost's themed 404 takes over - the controllers handle their own errors internally, and shared/express.js auto-registers sentry.errorHandler on every sub-app, so removing the trailing pageNotFound + handleHTMLResponse middlewares is safe - added an e2e-frontend smoke test that creates a page with slug 'gift' and asserts it renders at /gift/, plus a regression guard that /gift/<bad-token> still 302-redirects to the homepage Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
WalkthroughThis PR simplifies the gift-preview Express app by removing error handling middleware registrations (error-handler middleware and Sentry HTML error responses). The app now returns immediately after route setup without the previous error handling pipeline. Corresponding integration tests were added to verify that Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
ref https://linear.app/ghost/issue/BER-3662
Summary
Gift subscription redemption links live at
/gift/<token>. The sub-app atghost/core/core/server/web/gift-preview/is mounted viafrontendApp.lazyUse('/gift', ...)and currently captures every/gift*request — including the bare/giftand/gift/, which fall through toerrorHandler.pageNotFoundand produce a 404. That makes it impossible for a site owner to use/giftas a navigation link backed by a custom Ghost page orroutes.yamlentry.This PR removes the two terminating error-handler middlewares (and their now-unused imports) from the gift-preview sub-app. Tokenless requests now fall through into Ghost's normal frontend routing, where:
giftrenders if one exists,routes.yamlentry for/gift/is honored if one exists,The token-bearing redemption flow (
/gift/<token>,/gift/<token>/image) is untouched.Why this is safe
giftPreview,giftPreviewImage) handle their own errors viatry/catchand never propagate, so the sub-app's HTML error-handling stack was a dead path for any real request.shared/express.jsalready auto-registerssentry.errorHandleras the first error middleware on every sub-app, so removing the explicithandleHTMLResponse(sentry)does not drop Sentry coverage./:tokenand/:token/imagestill match tokened requests first, so the existing redemption and OG-image flows are byte-identical.Test plan
test/unit/server/web/gift-preview/controller.test.jsstill pass (controller is unchanged).test/e2e-frontend/default-routes.test.js:giftand assertsGET /gift/returns 200 with that page's title — proves the sub-app no longer intercepts tokenless requests.GET /gift/<unknown-token>still returns a 302 redirect to the homepage (existing controller fallback for invalid tokens — regression guard).pnpm lint:serverandpnpm lint:testclean.Manual verification
With
pnpm devrunning and thegiftSubscriptionslab flag enabled:GET /gift/<valid-token>→ 200 HTML with meta-refresh to/#/portal/gift/redeem/<token>(unchanged)GET /gift/<invalid-token>→ 302 to homepage (unchanged)GET /gift/<valid-token>/image→ 200 PNG (unchanged)GET /giftwith no/giftpage configured → themed Ghost 404 (was: bare sub-app 404)GET /gift/with a custom page at sluggift→ that page renders (was: 404)