feat(api): add proxy passthrough to OpenConnector#8
Merged
Conversation
The self-hosted open-connector backend added a `POST /v1/proxy/:service` passthrough (its "feat: impl proxy"), so the one call path OpenConnector still lacked relative to the hosted `Connector` is now available on the runtime. Add `open.proxy<T>(service, req, options?)` mirroring `Connector.proxy` — the same shared `ProxyRequest`/`ProxyResponse` types and the `connectionName` -> `x-oo-connector-alias` selector, with `proxy` joining the reserved members so a colliding service id only loses its path-2 sugar. Unlike the hosted gateway, the runtime requires `endpoint` to be a relative path beginning with `/` and answers `proxy_not_supported` for providers without an executor; both are documented on the method. Also add an optional `bodyEncoding: "base64"` to the shared `ProxyResponse` for binary bodies, refresh the README plus all five translations and `examples/open.ts`, and flip the `with-b` tsd fixture's `expectError(open.proxy(...))` to a positive assertion. Signed-off-by: Kevin Cui <bh@bugs.cc>
Summary by CodeRabbit
WalkthroughThis PR adds Sequence Diagram(s)sequenceDiagram
participant Caller
participant OpenConnector
participant RuntimeAPI
Caller->>OpenConnector: open.proxy(service, req, options)
OpenConnector->>RuntimeAPI: POST /v1/proxy/{service}
RuntimeAPI-->>OpenConnector: envelope(status, headers, data, bodyEncoding)
OpenConnector-->>Caller: ProxyResponse<T>
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/open.ts`:
- Around line 159-165: The `proxy` JSDoc in `Open` conflicts with the shared
`ProxyRequest.endpoint` documentation, which still suggests full URLs are
allowed. Update the shared `ProxyRequest` doc in `src/types.ts` to clearly state
the self-hosted/runtime restriction that `endpoint` must be a relative path
starting with `/`, or explicitly note the divergence from the hosted gateway so
callers are not misled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6546006b-3a7d-4611-bbf2-b8d5d2582aab
📒 Files selected for processing (11)
README.mddocs/README/README.fr.mddocs/README/README.ja.mddocs/README/README.ru.mddocs/README/README.zh-CN.mddocs/README/README.zh-TW.mdexamples/open.tsfixtures/consumer/with-b/index.test-d.tssrc/open.tssrc/types.tstest/open.test.ts
The shared `ProxyRequest.endpoint` doc advertised a path OR a full URL, but the self-hosted `OpenConnector` runtime rejects absolute URLs and requires a relative path beginning with `/`. Note that divergence on the shared type so a self-hosted caller reading it doesn't hit an avoidable runtime rejection. Addresses a CodeRabbit review comment on #8. Signed-off-by: Kevin Cui <bh@bugs.cc>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/types.ts (1)
79-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify
datatyping whenbodyEncodingis set.When
bodyEncoding: "base64"is present,dataholds an encoded string rather than the genericT. Since the runtime postsreqto/v1/proxy/{service}and returnsenvelope.datacast asProxyResponse<T>, and the test confirms "passes a binary body through: surfacesbodyEncoding: base64from the runtime" withdataas a base64 string, callers relying onTtyping may be misled for binary responses. Consider documenting this discrepancy inline, or typingdataasT | stringconditioned onbodyEncoding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types.ts` around lines 79 - 83, Clarify the ProxyResponse typing around binary payloads: when bodyEncoding is "base64", the runtime returns envelope.data as a base64 string rather than the generic T. Update the ProxyResponse type in types.ts and the related runtime/proxy handling so callers of the /v1/proxy/{service} flow can see that data is string-typed for binary responses, and keep the existing binary-body test expectations aligned with the ProxyResponse<T> shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/types.ts`:
- Around line 79-83: Clarify the ProxyResponse typing around binary payloads:
when bodyEncoding is "base64", the runtime returns envelope.data as a base64
string rather than the generic T. Update the ProxyResponse type in types.ts and
the related runtime/proxy handling so callers of the /v1/proxy/{service} flow
can see that data is string-typed for binary responses, and keep the existing
binary-body test expectations aligned with the ProxyResponse<T> shape.
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
1 participant
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.
The self-hosted
open-connectorbackend gained aPOST /v1/proxy/:servicepassthrough, soOpenConnectorcan finally offer the one call path it was still missing relative to the hostedConnector.open.proxy<T>(service, req, options?)mirrorsConnector.proxy: it reuses the sharedProxyRequest/ProxyResponsetypes and the sameconnectionName→x-oo-connector-aliasselector, andproxyjoins the reserved members so a service id that collides with it only loses its path-2 namespace sugar. Two runtime-specific behaviors are documented on the method: the endpoint must be a relative path beginning with/(the hosted gateway also accepts absolute URLs; this one rejects them), and providers without a proxy executor answerproxy_not_supported.Alongside the method:
ProxyResponsegains an optionalbodyEncoding: "base64"for binary upstream bodies (shared with the hosted client — additive and non-breaking).examples/open.tsdrop the old "exceptproxy" caveat and show the path-3 call.with-btsd fixture flipsexpectError(open.proxy(...))to a positiveexpectType, sinceproxyis now a real method (onlyusingstays hosted-only).open.proxytests covering the route, connection selector, binary passthrough, and error mapping.Full suite is green: lint, typecheck, 181 unit tests, and the type-acceptance matrix.