Skip to content

feat: server-render the request page so crawlers get the request - #118

Merged
rodrigopavezi merged 1 commit into
mainfrom
fix/req-scan-server-render-request
Jul 29, 2026
Merged

feat: server-render the request page so crawlers get the request#118
rodrigopavezi merged 1 commit into
mainfrom
fix/req-scan-server-render-request

Conversation

@rodrigopavezi

@rodrigopavezi rodrigopavezi commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Makes the request page serve the actual request in its HTML, instead of an empty skeleton. Top of the stack, on #117.

#116 stopped the 500s, but the page still served nothing useful: it fetched on the client, so the HTML was a skeleton with zero request data. For the SEO audit that started this, a 200 with an empty shell is barely better than the 500 it replaced.

page.tsx is now an async server component that awaits fetchRequest and passes the Channel to a new client component, request-details.tsx, which keeps everything genuinely browser-bound: PDF export, the JSON viewer, clipboard buttons, TimeAgo, useState. The relocation is verbatim — same markup, classNames, copy, table structure.

Verified against a local stand-in backend: the served HTML now contains the gateway, payee, payer, expected amount, payment reference, encryption status, and the transactions table. Before, none of it was there.

Plus generateMetadata, so these pages finally have a real title and description for search results and link previews.

Two findings worth reading

json-edit-react cannot be server-rendered

Its theme provider calls document.documentElement.style.setProperty("--jer-highlight-color", …) during render. The moment the details actually server-rendered, that threw ReferenceError: document is not defined, React bailed out to client rendering, and the HTML went straight back to being an empty shell — the twin of #116's bug, hidden until now because SSR had never got as far as rendering this component.

It is now loaded via next/dynamic with ssr: false. The raw JSON blob is not the indexable content, so nothing is lost.

Note this is a render-time touch of document, not an import-time one — which is why an import-level audit of the very same file came back clean. Worth remembering when auditing the next one.

A missing request still returns 200, not 404

It renders the not-found UI, but the status is wrong, and this is a framework limit rather than something left undone.

The route streams (Transfer-Encoding: chunked), so the HTTP status commits when the shell flushes — before the lookup resolves. notFound() can then only swap the rendered UI. Both placements were tried and measured:

Approach Result
notFound() from the page body 200
notFound() from generateMetadata 200

A real 404 needs the existence check to run before the response commits — i.e. middleware, with a Hasura call on every request. That is a bigger trade than this PR should make unilaterally, so I stopped and flagged it instead.

What I did instead addresses the actual harm: a missing request is served <meta name="robots" content="noindex, nofollow">, which is what stops soft 404s accumulating in the search index. Valid requests are not noindexed — checked both ways.

Implementation notes

  • The request lookup is wrapped in React's cache() keyed on the id, so generateMetadata and the page share one network call. The fetch-level cache cannot do this: graphql-request issues a POST, which Next's Data Cache does not dedupe, and the cache() inside graphQlClient is keyed on a fresh RequestInit per call.
  • A thrown fetch is deliberately left to propagate to the error boundary. A backend outage must not be reported as "this request does not exist" — that distinction was established in fix: stop every /request/<id> URL returning HTTP 500 #116 and is preserved here.
  • The two dependent queries (payments, SRF deployments) stay client-side: they hang off a payment reference derived from the request, they are the polling surface via refetchInterval, and keeping them there guarantees neither can affect the HTTP status or block the request's own content.
  • One deliberate behaviour change: the page no longer blocks on a full-page skeleton while payments load. The four payments-derived cells — Status, Balance, Modified, and the payments table — render their own inline loading state. Without that they would briefly display wrong values (Balance: 0, Status taken from the last transaction instead of "Paid") before the query lands. Everything else renders immediately from the server.

Verification

npm run check ✅, npm run build ✅, tsc --noEmit clean, zero SSR errors in the server log, and against a local stand-in backend:

Check Result
Request fields in served HTML ✅ present (were absent)
Missing request → noindex
Valid request → not noindexed
Missing request → 404 status ❌ 200 — framework limit, documented above

rodrigopavezi commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Comment thread src/app/request/[id]/page.tsx
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

The request page now renders request data on the server while retaining browser-dependent and polling behavior in a client component.

  • Adds request-specific metadata and noindex directives for missing requests.
  • Deduplicates metadata and page lookups within a render pass.
  • Loads the JSON editor client-side to avoid its render-time DOM access.
  • Shows inline loading states while payment-derived data loads.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/app/request/[id]/page.tsx Converts the route to an async server component, adds metadata generation, and shares the request lookup through React cache.
src/app/request/[id]/request-details.tsx Preserves interactive request details in a client component, keeps dependent queries client-side, and isolates the DOM-dependent JSON editor from SSR.

Sequence Diagram

sequenceDiagram
    participant C as Crawler or Browser
    participant P as Request Page
    participant F as fetchRequest
    participant D as RequestDetails
    C->>P: GET /request/:id
    P->>F: getRequest(id)
    F-->>P: Channel
    P->>D: Render id and Channel
    D-->>C: Server-rendered request HTML
    Note over D,C: Payment and SRF queries continue client-side
Loading

Reviews (4): Last reviewed commit: "feat: server-render the request page so ..." | Re-trigger Greptile

@rodrigopavezi
rodrigopavezi force-pushed the fix/req-scan-server-render-request branch from efcbe2a to 4781b4f Compare July 28, 2026 15:58
@rodrigopavezi

Copy link
Copy Markdown
Contributor Author

Fixed — thanks, this was a good catch, and it was specifically this PR that made it severe.

fetchRequest sent one document covering both the storage and storage_sepolia remotes. Hasura nulls the whole document when either errors, so moving that call onto the server render meant a sepolia hiccup took down a perfectly good mainnet request. The two remotes are now queried independently via Promise.allSettled.

The fix landed one PR down, in #117 (cbe483e), since it belongs with the other query-layer resilience work rather than above it — this PR inherits it.

The part worth reviewing is what null is now allowed to mean, because this PR gave it teeth: null drives notFound() + robots: noindex.

Case Result
Either remote returns the channel return it, even if the other failed
Both answered, neither has it null — a genuine not-found
Not found, but a remote failed throw
Both failed throw

The third row is the one that matters: "absent" and "unavailable" are indistinguishable there, and returning null would tell a crawler a possibly-live request is gone and noindex a real page. Throwing sends it to the error boundary, preserving the invariant from #116 that a backend failure is never reported as "not found". The error names the failed remotes.

Note the same pattern still exists in queries/transactions.ts and address-transactions.ts — combined storage documents. Left as follow-up; channel.ts was the urgent one because it is the only one on the SSR path.

@rodrigopavezi
rodrigopavezi force-pushed the fix/req-scan-server-render-request branch from 4781b4f to 6ceeb6d Compare July 29, 2026 01:51

@bassgeta bassgeta left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, a nice refactor on top of solving the problem 💯

rodrigopavezi commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Jul 29, 2:11 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 29, 2:15 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 29, 2:16 PM UTC: @rodrigopavezi merged this pull request with Graphite.

rodrigopavezi added a commit that referenced this pull request Jul 29, 2026
Every `/request/<id>` URL on scan.request.network returns **HTTP 500**. This fixes it. Bottom of a 3-PR stack; independently mergeable.

Came out of an SEO audit (Linear REQ-285) that listed 132 request URLs returning 5XX. It is not 132 broken pages — it is one bug, and the set is unbounded: a request created minutes ago 500s just the same.

## Root cause

`html2pdf.js` **0.10.2** — the version `package-lock.json` pins — invokes its UMD wrapper with a bare `self` identifier, evaluated before the inner `require()`s and with no `typeof self !== "undefined"` guard. Node has no global `self`, so *importing* it server-side throws. It was imported at module scope in `use-export-pdf.tsx`, whose only importer is `request/[id]/page.tsx` — which is exactly why that one route 500ed while every other returned 200.

Reproduced locally before the fix:

```
ReferenceError: self is not defined
    at 11378 (.next/server/app/request/[id]/page.js)
    at Object.t [as require] (.next/server/webpack-runtime.js)
```

The import now happens inside the export handler, so it only evaluates in a browser.

**Impact worth stating precisely:** Next.js bails out to client rendering after the SSR throw, so a real browser renders the page fine — users clicking through from the homepage were unaffected. Crawlers, shared links, hard refreshes and anything status-code-sensitive got a 500 with no content.

**Side benefit:** the route's page bundle drops **196 kB → 22.8 kB** and first-load JS **1.02 MB → 851 kB**, since html2pdf no longer ships eagerly.

## Why CI never caught this

`next build` compiles the route but never requests it — `page.tsx` has no `generateStaticParams`, so the build marks it `ƒ (Dynamic) server-rendered on demand`. `npm run check` is `biome check --write`, formatting only, and `--write` means it cannot fail on anything auto-fixable. There are no tests in the repo. PR #117 adds the request-time smoke check that would have caught it.

## Also in this PR

All found while tracing the above:

- **Error boundaries.** There were none — no `app/error.tsx`, no `global-error.tsx`, no root `not-found.tsx` (`app/not-found/page.tsx` is an ordinary route, not the convention file). Any render-time throw escaped to a raw 500. Added all three; neither leaks an error message or stack.
- **A missing request reported 200.** The page called `redirect("/not-found")` — a 307 to a page that itself returns 200, i.e. a soft 404 telling crawlers a nonexistent request exists. Now `notFound()`.
- **"Failed" was indistinguishable from "not found."** The page read only `isLoading`, never `error`, so a backend failure rendered as "not found". Now gates on the query's `status`: pending → skeleton, error → error state, success-and-empty → `notFound()`. `isLoading` was doubly wrong here — with react-query v5 it is `isPending && isFetching`, and no fetch happens during SSR, so it is **false** on the server and the skeleton guard never fired there.
- **Error states in the three "Recent …" widgets**, which ignored the `status` their hooks already return and rendered "No data" on failure. This is live today: the payments subgraph returns `no allocations`, so the home page currently reports empty rather than broken.
- **Two crashers guarded.** The fully-unguarded `extensionsData[0].parameters` path in `request-table.tsx` throws while evaluating arguments, and therefore *outside* the try/catch inside `calculateShortPaymentReference` — the likely cause of **#89**. And `BigInt(Number(gasUsed) * Number(gasPrice))` in `payment-table.tsx`, where a missing field makes `BigInt(NaN)` throw a `RangeError`; it now multiplies in BigInt, which also avoids losing precision above `Number.MAX_SAFE_INTEGER`.
- **Dropped `@react-pdf/renderer`**, imported nowhere. The lockfile is regenerated to match — worth noting because `npm ci` *fails* on a package.json/lockfile mismatch, so CI would have broken on its first job otherwise.

Likely also closes **#72** (an unindexed request showing as "404") now that error and not-found are distinct states.

## Verification

`npm ci` → `npm run check` → `npm run build`, then a real server:

| Route | Before | After |
|---|---|---|
| `/request/<id>` | **500** | **200** |
| `/`, `/requests`, `/payments` | 200 | 200 |
| unmatched route | — | 404 |
| `self is not defined` in log | present | **zero** |

## Known gap, fixed in #118

The page still fetches on the client, so the served HTML is a skeleton with no request data, and a nonexistent id returns 200 rather than 404. Both need a server-side fetch — that is #118.
@rodrigopavezi
rodrigopavezi changed the base branch from fix/req-scan-per-chain-resilience to graphite-base/118 July 29, 2026 14:12
@rodrigopavezi
rodrigopavezi changed the base branch from graphite-base/118 to main July 29, 2026 14:13
The previous commits stopped /request/<id> returning 500, but the page
still served nothing useful: it fetched on the client, so the HTML was a
skeleton with zero request data. An SEO audit flagged these URLs, and a
200 with an empty shell is barely better than the 500 it replaced.

page.tsx is now an async server component that awaits fetchRequest and
passes the Channel to a new client component, request-details.tsx, which
keeps everything genuinely browser-bound: PDF export, the JSON viewer,
the clipboard buttons, TimeAgo, useState. The relocation is verbatim —
same markup, classNames, copy and table structure.

Verified against a local stand-in backend: the served HTML now contains
the gateway, payee, payer, expected amount, payment reference, encryption
status and the transactions table. Before, none of it was there.

Two things this uncovered, both worth reading:

**json-edit-react cannot be server-rendered.** Its theme provider calls
`document.documentElement.style.setProperty("--jer-highlight-color", …)`
during render. Once the details actually server-rendered, that threw
`ReferenceError: document is not defined`, React bailed out to client
rendering, and the HTML went back to being an empty shell — the original
bug's twin, hidden until now because SSR had never got as far as
rendering this component. It is loaded via next/dynamic with ssr:false.
The raw JSON blob is not the indexable content, so nothing is lost.
Note this is a *render-time* touch of document, not an import-time one,
which is why an import-level audit of the same file came back clean.

**A missing request still returns 200, not 404.** It renders the
not-found UI, but the status is wrong, and this is a framework limit
rather than something left undone. The route streams
(Transfer-Encoding: chunked), so the status commits when the shell
flushes, before the lookup resolves; notFound() can then only swap the
UI. Tried and measured: notFound() from the page body → 200, and
notFound() from generateMetadata → also 200. A real 404 needs the
existence check to run before the response commits, which means
middleware and a Hasura call on every request — a bigger trade than this
commit should make unilaterally.

So the actual harm is addressed instead: a missing request is served
`<meta name="robots" content="noindex, nofollow">`, which is what stops
soft 404s accumulating in the search index. Valid requests are not
noindexed — checked both ways.

Also: generateMetadata gives these pages a real title and description
for search results and link previews, and the request lookup is wrapped
in React's cache() keyed on the id so generateMetadata and the page share
one network call. The fetch-level cache cannot do that here — graphql-
request issues a POST, which Next's Data Cache does not dedupe, and the
cache() inside graphQlClient is keyed on a fresh RequestInit per call.

The two dependent queries (payments, SRF deployments) stay client-side:
they hang off a payment reference derived from the request, they are the
polling surface via refetchInterval, and keeping them there guarantees
neither can affect the HTTP status or block the request's own content.
The four cells they feed — Status, Balance, Modified, and the payments
table — render their own inline loading state, so they no longer briefly
display wrong values (Balance 0, Status from the last transaction) before
the query lands.
@rodrigopavezi
rodrigopavezi force-pushed the fix/req-scan-server-render-request branch from 6ceeb6d to 8f0d5dd Compare July 29, 2026 14:14
@rodrigopavezi
rodrigopavezi merged commit 8decc6c into main Jul 29, 2026
4 checks passed
@rodrigopavezi
rodrigopavezi deleted the fix/req-scan-server-render-request branch July 29, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants