fix(sdk): preserve injected custom HttpClient across LilySdk.withConfig (Closes #442) - #563
Merged
David-patrick-chuks-02 merged 1 commit intoSep 6, 2026
Conversation
withConfig() dropped the caller-injected HttpClient: it always returned new LilySdk(merged), so children of an SDK built with a custom transport silently rebuilt a default fetch client from createFetchHttpClient. The fix distinguishes the two transports: - an explicitly injected HttpClient is shared with the derived instance, so custom transport behavior survives withConfig (issue Lilly-Protocol#442) - the default fetch client is NOT shared: it is rebuilt from the merged config so baseUrl/credential overrides are captured in the transport closure and actually reach the wire (issue Lilly-Protocol#405 semantics) Adds two regression tests: a child routes requests through the parent's injected client (request invoked, response returned), and a derived instance built without an injected client sends to the overridden baseUrl instead of the original host.
This was referenced Sep 6, 2026
[Bounty: $100] Stop
paginate from re-fetching the same page; advance on cursor metadata instead
#403
Closed
Closed
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
2 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.
Closes #442
Problem
LilySdk.withConfig()insrc/sdk.ts:103-130ends withreturn new LilySdk(merged)without passingthis.httpClient. When the source SDK was constructed with a customHttpClient, the derived instance silently drops it and rebuilds a default fetch client viacreateFetchHttpClient(this.config).Note: naively passing
this.httpClientunconditionally would trade this bug for the opposite one (issue #405): when no client was injected, the default fetch client captures the ORIGINAL baseUrl/credentials in its closure, so a derived instance'sbaseUrl/apiKeyoverrides would never reach the wire. This PR fixes #442 while preserving #405's routing semantics.Changes
src/sdk.ts
injectedHttpClient, set only when a client was explicitly passed to the constructorwithConfig()now returnsnew LilySdk(merged, this.injectedHttpClient)when a client was injected, andnew LilySdk(merged)otherwise — the default fetch client is rebuilt from the merged configtests/sdk-withConfig.test.ts
preserves an injected custom HttpClient across withConfig (issue #442): child created viasdk.withConfig({ apiKey: 'tenant2' })exposeshttpClient === mockHttpClientand routes an actual request through it (request recorded, response returned)rebuilds the default fetch client when none was injected (issue #405 semantics): a derived instance sends to the overriddenbaseUrl(tracked via injectedfetch), not the original hostVerification
npm run typecheck— cleannpm run test:unit— 875 passed, only the 4 pre-existing Windows path-resolution failures remain (zero new failures vs. baseline)prettier --checkandeslintclean on both touched files