refactor(linq): one HTTP client for the provider, used by every rail (W078)

[JemimahEkong]

W078 · One HTTP client for Linq

Closes #235

Problem

client.ts, onramp.ts and bills.ts each defined their own call() against the same provider, with the same auth and the same base URL. That is three copies, not the two the issue names. Written months apart, they drifted in exactly the details that decide what a user is told. For a full day the KYC screen could only say "rejected the request", because the copy serving it discarded the provider's code and its plain-text reason.

What changed

• src/linq/http.ts is now the only code that reaches the provider. linqCall() owns:
  • the auth header;
  • per-attempt timeouts and the shared deadline;
  • JSON and non-JSON error parsing, including code extraction;
  • the 429 retry and backoff;
  • failure logging.
• Per-call differences are arguments, not forks. timeoutMs covers the NIN check's 30s, rateLimitRetries covers the offramp's 429 retry, and deadline, query and auth cover the rest.
• LinqError, retryOnProviderFailure and TOTAL_BUDGET_MS move there. client.ts re-exports the first two, so no import anywhere had to change.
• client.ts, onramp.ts and bills.ts each lose their private client: 461 lines removed, 19 added.

What differed between the copies, and which behaviour won

Behaviour	client.ts (offramp)	onramp.ts	bills.ts	Now
Error code	code only	code, errorCode, error_code	not parsed	onramp's: all three keys, every rail
Error message keys	message	message, error, msg, detail, reason	message, error	onramp's
No readable reason	Linq returned <status>	Linq rejected the request, plus (<code>) when there is one	Linq rejected the request	onramp's (pinned by onramp.test.ts)
Short plain-text error body	kept as the message	kept if ≤200 chars and not HTML	discarded	onramp's
HTML or long non-JSON body	echoed as the message	refused	refused	refused, so a gateway's markup never reaches a screen
customerRef on an error	customerRef	customerRef, customer_ref	not read	onramp's
Retry-After	seconds only, used for our wait; not reported	seconds or HTTP date, reported as retryAfterSeconds; no wait	ignored	both: reported on every rail, and used for the wait where a call retries
429 retry	2 retries within the deadline	none	none	per call: offramp keeps 2; onramp and bills keep 0, so a NIN submission is never replayed
Per-call timeout	15s, capped by the deadline	15s; timeoutMs silently capped at 15s when no deadline was passed	15s	timeoutMs, capped only by the deadline. The NIN check's 30s now actually applies
Default deadline	35s	none	none	35s for every call. A single 15s attempt is unaffected
Network failure	502 carrying the raw error text (e.g. fetch failed)	502 Could not reach Linq, logged	502 Could not reach Linq, not logged	onramp's: no internals leak, and it's logged
Timeout	504, not logged	504, logged with elapsed time	504, not logged	onramp's
Failure logging	none	status plus the first 500 chars of the response body	none	onramp's, plus redaction of any 7+ digit run (NIN, phone, account, meter numbers), because bills now log too
Missing LINQ_API_KEY	500 not configured	502 Could not reach Linq (thrown inside the network catch)	same 502	500 not configured, with no request made
2xx with a non-JSON body	502 malformed response	502 response we could not read	502 response we could not read	502 Linq returned a response we could not read
2xx with an empty body	{}	null	null	{}. No caller treats either as valid, and {} doesn't throw a TypeError in offramp callers
URL building	${BASE}${path}	new URL(path, BASE), which drops any path prefix in LINQ_BASE_URL	same as onramp	concatenation plus encoded query: prefix kept, query encoded

Two of these are bug fixes rather than tie-breaks, so they're called out:

1. The NIN check's 30s timeout never took effect. The onramp client capped timeoutMs at its 15s default whenever no deadline was passed, and submitCustomerKyc passes none. A test now proves the check waits 30s.
2. A missing API key on the onramp and bills rails was reported as the provider being unreachable.

Acceptance criteria

• Exactly one call() implementation reaches the provider. A test scans src/ and asserts that only linq/http.ts mentions LINQ_BASE_URL, the provider host or X-API-Key. It also asserts no other Linq module defines a call(), and that the only other fetch in src/linq/ is coinPrice.ts, which reads Horizon, not Linq.

• src/__tests__/linq/ passes with no changes to its assertions. No existing test file was modified (git diff origin/main -- src/__tests__ touches only the new file).

• Every route returns the provider's code. This is tested twice:

  • all 13 provider calls across the three rails;
  • all 12 provider-backed API routes (8 under /ngn, 4 under /offramp), each driven through stubbed fetch and the real modules.

  GET /offramp/orders/:id needs a stored order and access token, so it's covered at the call level through getOfframpStatus.

• A short plain-text body survives and an HTML page does not, tested on each rail. An over-long plain body is refused too.

• The PR says what differed and which won. See the table above.

Validation

• npm test: 762 of 763 passing. The only failure is the pre-existing openapiCoverage test (get /fees undocumented), which is W082's job, as the issue notes.
  • Before this change: 710 of 711 passing, with the same failure.
  • The 52 new tests are in src/__tests__/linq/httpClient.test.ts.
• npm run typecheck, npx tsc --noEmit and npm run build all pass.
• Every test stubs global.fetch; no code path can reach the provider. The only NIN-shaped value used is 00000000000.

[drips-wave]

@JemimahEkong Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[Miracle656]

This is the PR I most wanted in this repo, and you had already rebased it onto a fix I pushed this morning.

Why it matters. src/linq/onramp.ts had its own call(), separate from src/linq/client.ts — two HTTP clients to one provider. I found that the hard way today: Linq started returning the existing customerRef on email_already_in_use, and I had to apply the same fix twice, because fixing only the shared client would have changed nothing on the onramp path. That is not a tidiness problem, it is a correctness one — the next provider change would have missed half the surface again.

And you carried that fix forward rather than clobbering it:

src/linq/http.ts:91    readonly existingCustomerRef?: string,
src/linq/http.ts:292   const existingCustomerRef = firstString(body["customerRef"], body["customer_ref"]);
src/linq/onramp.ts:139 const existing = err instanceof LinqError ? err.existingCustomerRef : undefined;

The guard tests are the part that makes this stick. only http.ts names the LINQ_BASE_URL variable, names the provider's host, names the X-API-Key header, and has no other call() or fetch() in the Linq modules but Horizon's price feed. Without those, a second client grows back the first time someone needs a one-off request, and nobody notices until the next provider change misses it. A refactor that cannot be undone by accident is worth far more than one that merely tidies up today.

The NIN property survives, which was the thing I checked hardest. Only response bodies are logged, never request bodies, and the reasoning is still written down where someone adding a catch will read it:

Log a request body. These bodies carry a NIN, a customer's name, email…

src/__tests__/linq: 210 passed, tsc --noEmit clean, verified locally against current main.

I pushed one commit to your branch. Four of your own guard assertions failed on my machine and not on CI:

- "linq/http.ts"
+ "linq\http.ts"

join uses the platform separator, and the assertions are written with forward slashes — so the guard was satisfied and the comparison was not. It passed on CI's Linux and failed on any Windows checkout, which is the worst way for a test to be wrong: the person who can see the failure is the one least able to tell it is spurious. Normalised with split(sep).join("/") where the path is built.

Same family as the CRLF problem #250 diagnosed yesterday. Worth remembering that this repo has Windows contributors.

Merging once CI confirms.

[Miracle656]

CI confirmed. Typecheck & build passes with the path fix. Integration tests fails identically on main — tests/integration/api.test.ts, expected undefined to be 2 / 5 / 2, the same three assertions — so it is pre-existing and not yours. Merging through it, as we do for reds that are not the contributor's.

Worth flagging separately: that integration suite has been red since at least 10-16, and it is testing the Postgres read path. Someone should look at it.