Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughThe ChangesOffers JSON response
Priority: ⚪ Not assessed Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The endpoint now returns its existing offers payload with the expected JSON content type and preserved caching behavior. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation For ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/web/src/app/api/(server)/offers/route.ts">
<violation number="1" location="packages/web/src/app/api/(server)/offers/route.ts:11">
P2: When `client.offers` resolves to a `ServiceError` (Lighthouse unreachable, or response body fails schema validation), this endpoint still responds HTTP 200 and stamps the error body with `Cache-Control: public, max-age=300`, so the error gets served as a successful 200 and cached publicly for 5 minutes. `client.offers` is typed `Promise<OffersResponse | ServiceError>` (packages/web/src/features/billing/client.ts, `requestLighthouse` returns `lighthouseUnreachable` with `statusCode: 500`), but this touched response never branches on that. Since this PR is rewriting the exact response construction, handle the error case: respond with the error's status code and skip the public cache header.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }); | ||
|
|
||
| return new Response(JSON.stringify(offers), { | ||
| return Response.json(offers, { |
There was a problem hiding this comment.
P2: When client.offers resolves to a ServiceError (Lighthouse unreachable, or response body fails schema validation), this endpoint still responds HTTP 200 and stamps the error body with Cache-Control: public, max-age=300, so the error gets served as a successful 200 and cached publicly for 5 minutes. client.offers is typed Promise<OffersResponse | ServiceError> (packages/web/src/features/billing/client.ts, requestLighthouse returns lighthouseUnreachable with statusCode: 500), but this touched response never branches on that. Since this PR is rewriting the exact response construction, handle the error case: respond with the error's status code and skip the public cache header.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/app/api/(server)/offers/route.ts, line 11:
<comment>When `client.offers` resolves to a `ServiceError` (Lighthouse unreachable, or response body fails schema validation), this endpoint still responds HTTP 200 and stamps the error body with `Cache-Control: public, max-age=300`, so the error gets served as a successful 200 and cached publicly for 5 minutes. `client.offers` is typed `Promise<OffersResponse | ServiceError>` (packages/web/src/features/billing/client.ts, `requestLighthouse` returns `lighthouseUnreachable` with `statusCode: 500`), but this touched response never branches on that. Since this PR is rewriting the exact response construction, handle the error case: respond with the error's status code and skip the public cache header.</comment>
<file context>
@@ -8,9 +8,9 @@ export const GET = apiHandler(async () => {
});
- return new Response(JSON.stringify(offers), {
+ return Response.json(offers, {
headers: {
'Cache-Control': 'public, max-age=300'
</file context>
|
hey, #1595 was a bit of a bogus issue, so going to close this PR. sorry about that |
2 participants
The public offers endpoint serializes a JSON body but currently serves it as
text/plain. This switches the response toResponse.json(...)while preserving the cache header.A route test now checks the Lighthouse request, JSON content type, cache header, and response body.
Fixes #1595
Tested:
text/plainas expectedgit diff --checkpassedThe full web suite could not complete on the available 1.9 GiB/no-swap runner; dependency installation exceeded its resource limit. This PR is being submitted with that limitation stated explicitly.
Note
Low Risk
Small public API response-header fix with no auth or data-model changes; behavior for JSON parsers is improved with regression test coverage.
Overview
Fixes the public
GET /api/offersresponse so clients seeapplication/jsoninstead oftext/plainwhile the body stays the same JSON payload.The handler now uses
Response.json(offers, …)instead of manually stringifying into a genericResponse, and still setsCache-Control: public, max-age=300. A focused Vitest route test asserts the Lighthouseofferscall (with install id), content type, cache header, and body; the unreleased changelog notes the fix.Reviewed by Cursor Bugbot for commit f834eb1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the public offers endpoint to return
application/jsoninstead oftext/plainusingResponse.json, preserving the existingCache-Controlheader.Written for commit f834eb1. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests