Add HttpAdapter interface for custom HTTP transport injection

[fcunha-recurly]

Summary

• Introduces HttpAdapter abstract base class with a single execute(method, url, headers, body) method that returns Promise<HttpResponse>
• Adds DefaultHttpAdapter (built-in implementation) which owns gzip/deflate decoding, Content-Length, keep-alive agents, timeout, and ECONNRESET retry — extracted from the previous Http.makeRequest and BaseClient logic
• Constructor injection: new Client(apiKey, { httpAdapter: adapter }) — validated as instanceof HttpAdapter, defaults to DefaultHttpAdapter
• Exports HttpAdapter, DefaultHttpAdapter, HttpMethod, and HttpResponse from the top-level recurly module
• Adds lib/testing.js entry point exposing HttpAdapterContract.runSuite for adapter authors to verify their implementation
• Updates MockClient, BaseClient.test.js, Http.test.js, and Pager.test.js to the new adapter-based transport

🤖 Generated with Claude Code

[epagerecurly]

Adversarial Review

Blocking

1. mock_client.js export shape break (test/mock_client.js)
Was module.exports = MockClient (the class directly). Now module.exports = { MockClient, HttpResponse }. Any test file doing const MockClient = require('.../mock_client') gets a plain object — new MockClient() throws "MockClient is not a constructor" at runtime with no compile-time warning. The PR updates several callers but all callsites in the repo need to be verified.

2. httpRequest.abort() is deprecated (lib/recurly/DefaultHttpAdapter.js, timeout callback)
request.abort() was deprecated in Node 14.1.0 (DEP0072) and is obsolete in Node 18+. On every timeout it emits a deprecation warning to stderr and doesn't cleanly release the socket. The replacement is httpRequest.destroy().

Advisory

3. Accept-Encoding silently missing from adapter contract (lib/recurly/BaseClient.js _buildHeaders)
Accept-Encoding was removed from the headers passed to execute(). DefaultHttpAdapter re-adds it internally, which is correct — but the HttpAdapterContract test suite has no test that verifies compression negotiation. Custom adapter authors have no documented signal that they're expected to handle this. They'll silently get uncompressed responses with no error.

4. instanceof HttpAdapter fails in bundled environments (BaseClient.js constructor)
In webpack/esbuild bundles or monorepos where the library loads twice, instanceof checks prototype identity across module copies. A valid adapter that correctly extends HttpAdapter from a different copy of the module gets rejected with a misleading error. Since the whole point of this PR is third-party adapter injection, this is a foreseeable failure mode. A duck-type fallback (typeof options.httpAdapter.execute === 'function') would be more robust.

[epagerecurly]

Adversarial review notes

• [BLOCKING] httpRequest.abort() removed in Node.js v22 — DefaultHttpAdapter.js calls httpRequest.abort() in the timeout callback. This was deprecated in Node.js v14 (DEP0136) and removed in v22. On Node.js v22+, calling it throws TypeError: httpRequest.abort is not a function, leaving the Promise permanently pending and leaking the socket. Fix: use AbortController and pass the signal option to transport.request().

• [BLOCKING] MockClient export shape change may have missed callers — test/mock_client.js changed from module.exports = MockClient (class as default export) to module.exports.MockClient = MockClient. Any test file still importing via const MockClient = require('./mock_client') will fail at runtime with "MockClient is not a constructor". The diff shows this was updated in the visible test files, but the PR description should confirm no caller was missed.

• [Advisory] Shared aborted flag leaks on ECONNRESET retry failure — The aborted variable is declared once per execute() call and shared across both the original request and the ECONNRESET retry. If the first request hits the timeout handler (aborted = true), and the retry subsequently fails for any reason, the retry's on('error') handler exits early via if (aborted) return — the rejection is silently swallowed and the Promise never settles.

Automated adversarial review by Claude. Actively looks for reasons the PR should not merge. Use your judgment.
Note: AI code review on recurly-app PRs will be triggered automatically by CI in the future — manual @claude review comments will not be needed.

[fcunha-recurly]

1. mock_client.js export shape break

All callers in the repo were updated in the same commit (Pager.test.js, BaseClient.test.js). Confirmed no missed callsites.

---

2. httpRequest.abort() removed in Node.js v22

Fixed — replaced with httpRequest.destroy(). Available on all supported Node versions (8–24) and our existing if (aborted) return guard in the error handler already swallows the resulting error event.

---

3. Accept-Encoding silently missing from adapter contract

By design. Gzip is optional for custom adapters. The contract suite intentionally does not require it and the README documents the behavior.

---

4. instanceof HttpAdapter fails in bundled environments

Acknowledged. This is a server-side SDK so bundled environments are not the primary target. Happy to revisit if it becomes a real issue.

---

5. Shared aborted flag leaks on ECONNRESET retry failure

This behavior predates this PR — the same logic was in Http.js. Agreed it's worth a follow-up but out of scope here.

[epagerecurly]

Code quality review (marvin:code-reviewer-code-quality:Agent)

Result: FAIL — 2 blocking, 5 advisory

Blocking

1. Duplicated jsonResponse test helper — test/recurly/BaseClient.test.js:1139, test/recurly/Pager.test.js:1615
The same 4-line helper is copy-pasted verbatim in both files. If HttpResponse's constructor changes, the two files drift independently.
Suggested fix: extract to a shared test/http_test_helpers.js and require it in both.

2. ECONNRESET retry logic has zero test coverage — lib/recurly/DefaultHttpAdapter.js:420-428
The retry path (reqOptions.retried = true; submitRequest()) covers a real production failure mode (keep-alive socket reset) with no unit test. A regression here silently breaks keep-alive requests.
Suggested fix: add a unit test that injects a transport that emits ECONNRESET on the first call and succeeds on the second, asserting the promise resolves and the transport was called twice.

Advisory

3. Flaky "network failure" test — lib/recurly/HttpAdapterContract.js:972-983
Connects to http://127.0.0.1:1/test to provoke a rejection. Port 1 is not guaranteed to be refused on all OSes or CI sandbox configurations.
Suggested fix: spin up a real server, capture its port, close it, then connect to the now-closed port — guaranteed ECONNREFUSED.

4. TimeoutError type leaks through the transport contract — lib/recurly/DefaultHttpAdapter.js:305, 417
DefaultHttpAdapter throws an application-layer TimeoutError. Custom adapters throwing a plain Error on timeout produce a different type; BaseClient doesn't normalize transport rejections. Once the interface is public this contract is hard to change.
Suggested fix: either have BaseClient._makeRequest normalize transport rejections into TimeoutError, or add TimeoutError to the contract suite so all adapters are required to throw it.

5. ApiError used for constructor argument validation — lib/recurly/BaseClient.js:164
ApiError models HTTP API response errors; a bad constructor argument should throw TypeError.
Suggested fix: throw new TypeError('options.httpAdapter must be an instance of HttpAdapter')

6. Dead || with identical operands in Pager test — test/recurly/Pager.test.js:1685-1686
url.includes('...cursor=1234567890') || url.includes('...cursor=1234567890') — both sides identical, likely a copy-paste error.
Suggested fix: remove the duplicate operand, or supply the correct second URL if a variant was intended.

7. DefaultHttpAdapter unnecessarily exported from lib/testing.js — lib/testing.js:4,7
The testing entry point exists for adapter authors to verify their implementations via the contract runner — they don't need the default adapter there.
Suggested fix: remove DefaultHttpAdapter from lib/testing.js.