Repository navigation
feat: support OkHttp application interceptors - #899
sylvesterkaczmarek wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f687cddfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9c53d59 to
c8d9d59
Compare
|
I resolved this PR against current main while preserving the intended fix and newer upstream behavior. The branch is now mergeable and no longer has a base conflict. Could a maintainer please re-review it when convenient? @codex review |
Resolved against current OkHttp transport tests.
c8d9d59 to
bf429d1
Compare
|
Thank you for the work on this proposal. #1280 has merged and includes the application-interceptor support proposed here, with Closing this PR as superseded by the merged implementation. The change is available on |
|
Thanks for the update and for linking the merged implementation and docs. Glad to see application-interceptor support land upstream, including the existing-client option. Appreciate the review and the team taking this forward! |
Summary
Add application-interceptor support to the SDK's public OkHttp transport builder so users can customise requests and responses without copying the SDK's
HttpClientimplementation.Fixes #339.
Problem
The SDK currently exposes several OkHttp transport settings, but there is no supported way to register an OkHttp
Interceptor. The documented workaround is to copy the SDK's OkHttp transport implementation and maintain a local fork just to add interceptors.That is unnecessarily heavy for common use cases such as request tagging, custom observability, organisation-specific headers, response inspection, and integration with existing OkHttp middleware.
Fix
Add a Java-callable builder method:
Configured interceptors are registered as standard OkHttp application interceptors when the underlying transport is built. Multiple calls preserve registration order, matching OkHttp's normal semantics.
The existing public construction path can then use the customised transport directly through
ClientOptionsandOpenAIClientImpl, without copying SDK source.Regression coverage
Extended
OkHttpClientTestwith a WireMock-backed test that:Validation
mainat81134d727946fb84cc48db98e6c8c53c8a8d22ab;OkHttpClient.kt;Full repository validation is left to GitHub Actions.
Risk
Low. No interceptor is installed by default, so existing clients behave exactly as before. The new behaviour is entirely opt-in and delegates interceptor semantics to OkHttp.