Skip to content

fix(clk-gateway): log unhandled route errors, harden ZyFi failure paths - #125

Merged
aliXsed merged 2 commits into
mainfrom
fix/clk-gateway-zyfi-error-logging
Aug 5, 2026
Merged

fix(clk-gateway): log unhandled route errors, harden ZyFi failure paths#125
aliXsed merged 2 commits into
mainfrom
fix/clk-gateway-zyfi-error-logging

Conversation

@Douglasacost

@DouglasacostDouglasacost commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Rebased onto main. Scope narrowed substantially: the original version of this PR duplicated #118, which had already landed. Everything #118 covers has been dropped. What remains is additive only.

1. Unhandled route errors are never logged

src/index.ts — the global error middleware returns a 500 to the client but never logs:

constmessage=errinstanceofError ? err.message : String(err);res.status(500).json({error: message});

No unexpected error, on any route, leaves a server-side trace. Now logs message and stack for 500s, and handled HttpErrors at warn level with method and URL. #118 did not touch this.

2. Network-level failures are indistinguishable from HTTP rejections

A DNS, TLS, or connection-reset failure means the request never reached ZyFi, so there is no status or body to report. Previously this surfaced as an unhelpful generic throw. Now reported explicitly as a network failure.

3. response.json() ran before the ok check

This is a latent gap in #118:

constresponseBody=awaitresponse.json();// <-- before the ok checkif(!response.ok){ ... }

If a proxy or load balancer in front of ZyFi returns HTML or an empty body — a 502/504 — response.json() throws a SyntaxError and the status is discarded, reproducing exactly the blind spot #118 set out to close. The body is now read as text first and parsed after, so the status survives a non-JSON response.

The operation label convention from #118 is preserved throughout.

Verification

  • npx tsc --noEmit — clean.
  • npx jestroutes/names.integration.test.ts passes (7 tests).
  • src/helpers.test.ts fails, but pre-existing and unrelated: verified it fails identically on a clean checkout. Jest does not load .env, so SERVICE_ACCOUNT_KEY is undefined and JSON.parse throws at import in src/setup.ts:30. Left untouched.

Implementation note: in helpers.ts the bare type Response resolves to Express'sResponse, not fetch's, hence Awaited<ReturnType<typeof fetch>>.

Separately — a deployment lead

#118 removed the zyfiSponsoredUrl: ${...} log from helpers.ts. On current main the only place that string is still logged is testPaymaster.ts, a standalone script. A gateway emitting that line is therefore not running post-#118 code — worth checking whether the deployed build predates 2026-07-08.

Builds on #118, which added ZyFi status and body logging. Three gaps remain.
1. The global Express error middleware returns a 500 to the client but never
logs, so no unexpected error on any route leaves a server-side trace. Log
the message and stack for 500s, and log handled HttpErrors at warn level
with method and URL.
2. `fetchZyfiSponsored` did not distinguish a network-level failure (DNS, TLS,
connection reset) from an HTTP rejection. The request never reaches ZyFi in
that case, so there is no status or body to report; say so explicitly.
3. `await response.json()` ran before the `response.ok` check, so a non-JSON
error body — an HTML or empty response from a proxy or load balancer in
front of ZyFi — threw a SyntaxError that discarded the status and left the
same blind spot #118 set out to close. Read the body as text first, then
parse, so the status survives a non-JSON response.
Keeps the `operation` label convention introduced in #118.
@Douglasacost
Douglasacostforce-pushed the fix/clk-gateway-zyfi-error-logging branch from de38254 to cf007dbCompareJuly 30, 2026 15:00
@DouglasacostDouglasacost changed the title fix(clk-gateway): surface Zyfi paymaster and unhandled route errorsfix(clk-gateway): log unhandled route errors, harden ZyFi failure pathsJul 30, 2026
@github-actions

github-actionsBot commented Jul 30, 2026

Copy link
Copy Markdown

LCOV of commit b38f7ef during checks #762

Summary coverage rate:
lines......: 26.5% (939 of 3550 lines)
functions..: 27.0% (150 of 555 functions)
branches...: 27.1% (164 of 605 branches)
Files changed coverage rate: n/a

@Douglasacost

Copy link
Copy Markdown
CollaboratorAuthor

@aliXsed could you check this one 🙏

@aliXsed
aliXsed merged commit 91971ba into mainAug 5, 2026
3 checks passed
@aliXsed
aliXsed deleted the fix/clk-gateway-zyfi-error-logging branch August 5, 2026 23:16
Sign up for freeto 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

@Douglasacost@aliXsed