Merged
Conversation
Removes axios and axios-retry dependencies in response to the axios npm supply chain compromise. HTTP requests now use the native fetch API (Node 18+ / browsers) with undici's ProxyAgent for proxy support. - Rewrote HttpSender to use fetch with AbortSignal.timeout - Added ProxyConfig interface to types.ts (replaces AxiosProxyConfig) - Updated ClientBuilder.withProxy() to build a proxy URL string - Renamed AxiosLikeResponse to HttpResponse in buildSmartyResponse.ts - Added injectable fetchFn parameter for unit test mocking - Added integration tests using a local HTTP server
…ging - Add withProxy(url) single-arg overload for simpler proxy configuration - Narrow header types from Record<string, unknown/any> to Record<string, string> - Add debug logging for JSON parse failures in HttpSender - Add comment explaining undici dispatcher type bypass - Declare Node >=18.0.0 engine requirement (fetch is built-in from v18)
- Remove silent swallow of JSON parse errors in HttpSender - Narrow buildSmartyResponse error field from `any` to `string | Error` - Widen Response.error to `Error | string | null` to match actual usage - Make MockSenderWithStatusCodesAndHeaders.send() return Promise<Response> - Remove CompatibleMockSender wrapper and mock interfaces from types.ts
… tighten types RetrySender now catches rejected responses from the sender chain and converts them back to return values for retry evaluation. Mock sender updated to throw on >= 400 to match real HttpSender behavior. Removed ProxyConfig interface in favor of inline type, typed dispatcher as undici.Dispatcher, and cleaned up let -> const in retry tests.
…n, tighten debug type
Narrow Response.error back to Error | null (was Error | string | null) by wrapping string errors in buildSmartyResponse's new normalizeError helper. Validate baseUrl is an absolute HTTP(S) URL before constructing a URL object. Safely wrap non-Error throwables in the fetch catch block.
camiblanch
requested changes
Apr 2, 2026
…rrors, use dynamic import - Preserve HTTP status codes when parseResponseBody throws (e.g. malformed JSON on a 4xx/5xx response) instead of defaulting to status 0 - Split proxy init into two try/catch blocks so invalid proxy URLs get a clear message instead of the misleading "install undici" error - Switch from require() to await import() for undici so proxy works in ESM contexts; store promise and await in send()
smartymanwill
requested changes
Apr 2, 2026
| try { | ||
| return await this.inner.send(request); | ||
| } catch (error) { | ||
| if (error && typeof error === "object" && "statusCode" in error) { |
Contributor
There was a problem hiding this comment.
There's a slight behavioral change here. trySend() converts thrown error responses into resolved return values instead of throwing them (as it did previously). For example, with sender.send(request).catch(handleError), if retry attempts are exhausted for a 500-level response, the code may silently receive an error response through the success path. Is that change intentional?
Contributor
Author
There was a problem hiding this comment.
Fixed in a way. After retry is exhausted, we will return the error.
| if (username && password) { | ||
| auth = `${encodeURIComponent(username)}:${encodeURIComponent(password)}@`; | ||
| } | ||
| this.proxy = { url: `${protocol}://${auth}${hostOrUrl}:${port}` }; |
Contributor
There was a problem hiding this comment.
Super minor, but for plain JS implementations (without TS checking), if protocol is undefined at runtime, this produces undefined://host:port which will silently pass through and fail later in ProxyAgent (the Axios code stored it as a plain object so this wasn't an issue.). Something like if (!protocol) throw new Error(...) could catch this early.
…unknown errors - Handle statusCode 0 (network errors) explicitly, keeping the original error (e.g. "fetch failed") instead of overwriting with "unexpected error" - Fall back to the existing error message in the default case when the response payload has no API error details
trySend() intentionally converts thrown error responses to return values so the retry loop can inspect statusCode and decide whether to retry. But after the loop, error responses were silently returned through the success path. Now re-throws if the response has an error or still carries a retryable status code.
Without this guard, calling withProxy(host, port) without a protocol from plain JS produces "undefined://host:port" which silently passes through and fails later in ProxyAgent with a confusing error.
Replace the compound check (response.error || statusToRetry.includes) with a straightforward status code check: throw if the response is not a success. This also fixes non-retryable errors like 422 that were silently returning through the success path.
smartymanwill
approved these changes
Apr 3, 2026
Contributor
smartymanwill
left a comment
smartymanwill
left a comment
There was a problem hiding this comment.
This looks great. Nice work!
camiblanch
approved these changes
Apr 3, 2026
Contributor
camiblanch
left a comment
camiblanch
left a comment
There was a problem hiding this comment.
Looks great! Everything seems good to go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Get rid of Axios dependency.