Repository navigation
fix(web): answer a failed profile, community or wave lookup as retryable, not 404 - #2001
Conversation
…ble, not 404 A server lookup of an account, community or wave post that failed upstream (RPC error, rate limit, timeout) was treated as "does not exist": the page answered 404, telling readers and crawlers a live page was gone. - prefetchRequired: returns the record, calls notFound() only when the lookup succeeded and found nothing (or a node asserted it does not exist). It otherwise marks the response lookup-unavailable and throws SsrLookupUnavailableError. Same React Query fetch as prefetchQuery. - Profile, community and wave pages use it for their record lookup. The profile section page answers an unknown section before any lookup. - ssr-degraded.js sends a lookup-unavailable response that ended in a server error as 503 with Retry-After (still private, no-store). Headers handed to writeHead can no longer override a degraded response's Cache-Control. The minute log line carries a short sample of the failing errors. - Profile, community and wave segments get an error module with a retry inside the layout instead of the global 500. Client render errors are still reported, through the lazy Sentry facade. - The server Sentry config drops SsrLookupUnavailableError. - isHiveNotFoundError also recognises the asserts for an account name no account can have, so those URLs stay cacheable 404s. Closes #2000
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoTreat failed profile, community, and wave lookups as retryable
AI Description
Diagram
High-Level Assessment
Files changed (37)
|
 [Medium impact] The PR changes failure responses across profile, community, and wave page lookups. The PR still needs the existing wave lookup fix before merging.
 |
Code Review by Qodo
1. New lookup tests bypass type checking
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| ({ queryKey: ["accounts", "full", username], queryFn: lookup(accountOutcome) }) as any | ||
| ); | ||
| vi.mocked(getCommunityQueryOptions).mockImplementation( | ||
| (name?: string) => | ||
| ({ queryKey: ["communities", "single", name], queryFn: lookup(communityOutcome) }) as any |
There was a problem hiding this comment.
1. New lookup tests bypass type checking 📘 Rule violation ⚙ Maintainability
The new lookup regression spec uses as any, Promise<any>, and arrays containing Promise<any> route loaders instead of explicit types. These casts and annotations suppress checking across mocked query options, route imports, and render helpers in the added test paths.
| vi.mocked(getAccountFullQueryOptions).mockImplementation( | ||
| (username?: string) => | ||
| ({ queryKey: ["accounts", "full", username], queryFn: lookup(accountOutcome) }) as any | ||
| ); | ||
| vi.mocked(getCommunityQueryOptions).mockImplementation( | ||
| (name?: string) => | ||
| ({ queryKey: ["communities", "single", name], queryFn: lookup(communityOutcome) }) as any |
There was a problem hiding this comment.
2. Lookup tests hardcode react query keys 📘 Rule violation ⚙ Maintainability
The added query-option mocks construct keys with array literals such as `['accounts', 'full', username] and ['entry', author, permlink] rather than obtaining them from shared QueryKeys` constants. These keys are passed through the mocked query builders and consumed by the real query client in the regression tests, so the new query paths bypass the repository's centralized key definitions.
| // Kept apart from ctx.reason: an earlier mark under another reason must not | ||
| // stop a failed record lookup from going out as a 503. | ||
| if (why === LOOKUP_UNAVAILABLE && !ctx.res.headersSent) ctx.unavailable = true; |
There was a problem hiding this comment.
4. Lookup failures disappear from reason counts 🐞 Bug ◔ Observability
mark() sets ctx.unavailable for a required-lookup failure but leaves an earlier ctx.reason unchanged. If an optional prefetch has already marked the request, the resulting 503 is counted under that earlier reason; after headers are sent, the existing reason also prevents a late lookup-failure count.
Agent Prompt
## Issue description
A required-lookup failure can produce a 503 without appearing in the per-reason lookup-unavailable counts when another prefetch marked the request first.
## Fix Focus Areas
- apps/web/ssr-degraded.js[84-101]
- apps/web/ssr-degraded.js[157-180]
## Recommended Fix
Track the lookup-failure reason independently of the first degradation reason, and include it in the appropriate sent or late count without counting the response twice. Add coverage for an earlier prefetch mark followed by a required-lookup failure.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| prefetchRequired( | ||
| EcencyEntriesCacheManagement.getEntryQueryByPath(authorName, permlink) | ||
| ) as Promise<WaveEntry>, |
There was a problem hiding this comment.
If the first node returns null for an existing wave, getPostQueryOptions checks other nodes through verifyPostOnAlternateNode. That helper catches failed checks and returns null. The new prefetchRequired call then sees a successful empty result and calls notFound(), sending a cacheable 404 instead of a retryable error.
This leaves part of the intended fix incomplete. Preserve failed checks through the SDK helper so this case reaches SsrLookupUnavailableError.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Required lookup classification apps/web/src/core/react-query/query-helpers.ts, apps/web/src/utils/hive-not-found-error.ts, apps/web/src/specs/core/react-query/prefetch-required.spec.ts, apps/web/src/specs/utils/hive-not-found-error.spec.ts |
prefetchRequired returns available data, calls notFound() for successful empty results or recognized Hive not-found errors, and marks other failures before throwing SsrLookupUnavailableError. Hive not-found matching now includes additional invalid account-name responses. |
Profile, community, and wave page integration apps/web/src/app/(dynamicPages)/profile/..., apps/web/src/app/(dynamicPages)/community/..., apps/web/src/app/waves/[author]/[permlink]/page.tsx, apps/web/src/specs/app/profile-community-lookup-unavailable.spec.ts, apps/web/src/specs/app/*-ssr-stream.spec.tsx, apps/web/src/specs/setup-any-spec.ts |
Required account, community, and wave lookups use prefetchRequired instead of nullable prefetches with local notFound() checks. The profile section page validates supported sections before lookup. Route tests and streaming fixtures cover the updated calls. |
SSR response status and diagnostics apps/web/ssr-degraded.js, apps/web/src/specs/ssr-degraded.spec.ts, apps/web/sentry.server.config.ts, apps/web/src/specs/utils/sentry-config-wiring.spec.ts |
Marked lookup-unavailable responses that end with a 5xx status are sent as 503 with Retry-After: 30 and private no-store caching. The handler samples distinct failure details for logs. Server Sentry drops SsrLookupUnavailableError events. |
Lookup error boundaries and retry apps/web/src/app/(dynamicPages)/profile/[username]/error.tsx, apps/web/src/app/(dynamicPages)/community/[community]/error.tsx, apps/web/src/app/waves/[author]/[permlink]/error.tsx, apps/web/src/app/global-error.tsx, apps/web/src/features/shared/lookup-unavailable/*, apps/web/src/features/i18n/locales/en-US.json, apps/web/src/specs/app/lookup-unavailable-error-boundary.spec.tsx |
The profile, community, and wave segments use the shared client error component. It reports errors without a digest to Sentry and refreshes the route and resets the boundary on retry. The component renders translated error and retry text. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Sequence Diagram(s)
sequenceDiagram
participant ProfilePage
participant prefetchRequired
participant QueryClient
participant ssr-degraded.js
participant HTTPResponse
ProfilePage->>prefetchRequired: request required data
prefetchRequired->>QueryClient: run timed query
QueryClient-->>prefetchRequired: return query result
prefetchRequired->>ssr-degraded.js: mark lookup-unavailable with detail
prefetchRequired-->>ProfilePage: throw SsrLookupUnavailableError
ssr-degraded.js->>HTTPResponse: send 503 with Retry-After and no-store headers
Merge Risk: 
82d7b
-
No actionable issue remains; the change is ready to merge after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: failed profile, community, or wave lookups now produce a retryable outcome instead of a 404 response. Linked Issues check Directly linked issue #2000is the only active target. Its coding requirements are covered at the reviewed head.
[#2000] Lookup outcome helper: apps/web/src/core/react-query/query-helpers.ts defines …Out of Scope Changes check Changes that go beyond the page list in #2000are connected to its objectives:
- Wave page (apps/web/src/app/waves/[author]/[permlink]/page.tsx) and wave error module: these apply the same failed-loo…Full details: Linked Issues check
-
Explanation
Directly linked issue
#2000is the only active target. Its coding requirements are covered at the reviewed head.[
#2000] Lookup outcome helper: apps/web/src/core/react-query/query-helpers.ts defines prefetchRequired. Found data is returned. A successful empty result, or a Hive not-found error (isHiveNotFoundError), calls notFound(). An error or timeout calls markSsrDegraded('lookup-unavailable', detail) and throws SsrLookupUnavailableError. It uses the same qc.prefetchQuery call as prefetchQuery, so the layout and page share one fetch.[
#2000] Page switches: the summary lists the profile index, [section] (both branches), followers, following, trail, communities, permissions, settings, referrals, insights, the community index, [tag] (both branches), activities, subscribers and roles as using prefetchRequired. Unknown [section] values are rejected before lookup.[
#2000] Status handling: apps/web/ssr-degraded.js rewrites a lookup-unavailable response with a 5xx status to 503, sets Retry-After: 30 and Cache-Control: private, no-store, and drops conflicting passed headers. 404 and 200 keep their status. The per-minute log line adds up to three sampled error details.[
#2000] Error modules and Sentry: the profile, community and wave segments re-export LookupUnavailableError. The summary states that client errors without a digest are reported through the lazy Sentry facade. The server beforeSend drops SsrLookupUnavailableError.Acceptance specs named in
#2000exist in the PR. I did not run them. This result relies on the code read at the reviewed head and on the author's reported test runs.Residual: a late mark on a client-navigation RSC response keeps the route Cache-Control.
#2000does not require a 503 for that case.
Full details: Out of Scope Changes check
-
Explanation
Changes that go beyond the page list in
#2000are connected to its objectives:- Wave page (apps/web/src/app/waves/[author]/[permlink]/page.tsx) and wave error module: these apply the same failed-lookup rule to waves.
#2000does not list waves. The change is not unrelated, because it uses the same prefetchRequired and lookup-unavailable mechanism. The PR states that waves are outside the issue's stated profile and community scope. The author should confirm that this extension is intended. - Post pages (/@author/permlink): unchanged. This respects the
#2000exclusion. - apps/web/src/utils/hive-not-found-error.ts (NOT_FOUND_ASSERTS): classifies invalid or oversized account-name errors as not-found. prefetchRequired relies on isHiveNotFoundError to choose notFound() for a not-found assert, so this supports the
#2000criterion. - ssr-degraded.js log-sample change: requested by
#2000(per-minute log line with a failing-error sample). - global-error.tsx comments, en-US.json strings, setup-any-spec.ts SDK mock builders and the render-spec mock updates: supporting documentation, translations, and test scaffolding for the listed changes.
- Spec files: they test the listed acceptance criteria.
No change was found that is unrelated to
#2000or to its failed-lookup scope. - Wave page (apps/web/src/app/waves/[author]/[permlink]/page.tsx) and wave error module: these apply the same failed-lookup rule to waves.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 15 functions across 36 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the lookup trail,
If data hides, it will not fail.
A missing page gets 404,
A shaky call can try once more.
Refresh, reset, and hop away!
Comment @coderabbitai help to get the list of available commands.
- The segment error module catches any error in its routes, so its copy names no cause. - A failed record lookup is counted as lookup-unavailable even when a layout's optional prefetch marked the request first. - Control characters in an upstream error message are removed before the sample reaches the log line.
Closes #2000
What changed
A server lookup of an account, community or wave post that failed upstream (RPC error, rate limit, timeout) was answered as "does not exist": the page sent a 404 for a live profile. Now a failed lookup is told apart from a missing record.
prefetchRequired(core/react-query/query-helpers.ts), next toprefetchQueryand on the same React Query fetch (a layout and page asking for one key still share one upstream call):notFound()(unchanged 404, still cacheable);lookup-unavailableand throwsSsrLookupUnavailableError.[section](both branches), followers, following, trail, communities, permissions, settings, referrals, insights; community index,[tag](both branches), activities, subscribers, roles; the wave page.[section]now answers an unknown section before any lookup.ssr-degraded.js: alookup-unavailableresponse that ended in a server error is sent as503withRetry-After: 30, stillprivate, no-store. A 404 or 200 keeps its status. Headers passed towriteHeadcan no longer override a degraded response'sCache-Control. The per-minute log line ends with a short sample of the failing errors (with the node when the SDK names it, control characters removed). A failed record lookup is counted under its own reason even when a layout marked the request first.features/shared/lookup-unavailable): a retry (refresh + reset) inside the layout instead of the global 500. It renders on client-side navigation and on the profile sub-pages that stream through their ownloading.tsx; on routes without a Suspense boundary a full page load gets the 503 instead. Client render errors are still reported, through the lazy Sentry facade (no SDK on the eager bundle). It catches any error in those routes, so its copy names no cause.SsrLookupUnavailableErroris dropped inbeforeSend; it is an expected upstream condition, counted in the log line.isHiveNotFoundError: an account name no account can have (too long, or invalid characters, as answered by both the chain and hivemind) is a not-found answer, so such URLs stay cacheable 404s.Behaviour by layer
no-store)Retry-After,no-store; an origin with a fresh cached copy can serve it stale while an edge retry can try another originloading.tsxno-store)Known residual (same class as before): a client-navigation (RSC) response writes its head before the page resolves, so a failure there is a "late" mark and keeps the route's Cache-Control, as a not-found did before. The retry sends a different router state, so it does not hit that entry.
Post pages (
/@author/permlink) are unchanged: a failed lookup there already renders the client-side fallback that re-reads the chain from the browser.Test first
Command for each:
pnpm --filter @ecency/web exec vitest run <spec>(paths relative toapps/web).src/specs/core/react-query/prefetch-required.spec.tspromise resolved "undefined" instead of rejecting(failed and timed-out lookups),promise resolved "null" instead of rejecting(missing)src/specs/app/profile-community-lookup-unavailable.spec.tsexpected Error: NEXT_NOT_FOUND to be an instance of SsrLookupUnavailableErroron all 17 profile/community failed-lookup cases, then the wave casesrc/specs/ssr-degraded.spec.tsexpected 500 to be 503(3 cases), log line without the error sample,expected 'public, s-maxage=300' to be 'private, no-store'src/specs/app/lookup-unavailable-error-boundary.spec.tsxUnable to find an element with the text: lookup-unavailable.title, Sentry not called for a client error, segmenterror.tsxmissingsrc/specs/utils/sentry-config-wiring.spec.tsexpected { …(4) } to be nullsrc/specs/utils/hive-not-found-error.spec.tsexpected false to be truefor the impossible account name assertsMutations (each reverted after):
prefetchRequiredreturning undefined on failure (20 failures), every error state answerednotFound()(19), timeout answerednotFound()(1), 503 rewrite removed (3), one page reverted toprefetchQuery+notFound()(1), Sentry filter removed (1), header strip removed (1).Full web suite: 584 files, 5979 tests passed.
tsc --noEmitclean, eslint clean on changed files, icon/slim-entries/origin-config audits pass.Also verified with
next dev, the production preload and a stub RPC node: failed lookup on the profile index and a community feed answered 503 +Retry-After: 30+private, no-store; a missing account answered 404; a healthy lookup answered 200 with unchanged headers.The three render specs that stub
prefetchQuerynow stubprefetchRequiredthe same way. The global SDK spec mock gained the profile and ranked feed builders the pages build their queries with.