feat: added support for proxy#3
Conversation
WalkthroughThis pull request adds comprehensive proxy support to the Enkryptify client. It introduces a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/enkryptify.ts`:
- Around line 113-125: The code uses a brittle fallback proxy URL
(DEFAULT_PROXY_URL / `https://proxy-poc-black.vercel.app`) when neither
`config.proxy?.url` nor `process.env.ENKRYPTIFY_PROXY_URL` is set, which can
silently route traffic to a preview deployment; update the initialization in
`proxyUrl`/`EnkryptifyProxy` use to either require an explicit proxy URL (fail
fast with a clear error when `config.proxy?.url` and `ENKRYPTIFY_PROXY_URL` are
both missing) or replace `DEFAULT_PROXY_URL` with an official stable domain, and
ensure any error message references `proxyUrl`/`config.proxy` so consumers know
to set `ENKRYPTIFY_PROXY_URL` instead of relying on the preview host.
In `@src/proxy.ts`:
- Around line 181-204: The `#send` method currently returns fetch Response
verbatim, so callers never observe the advertised
ProxyError/ProxyValidationError types; change `#send` to detect proxy-originated
errors (e.g., check for a proxy-specific response header such as X-Proxy-Error
or the proxy JSON shape {error:{code,message}} on non-2xx responses), parse the
body, and convert those into the exported ProxyError or ProxyValidationError
types before throwing, while letting transport (fetch) exceptions continue to
bubble and leaving non-proxy upstream responses untouched; update the detection
logic inside the `#send` implementation (the method that calls fetch against
this.#proxyUrl and currently returns response) to parse response.json() when the
discriminator is present, construct and throw the correct
ProxyError/ProxyValidationError, and keep other responses returned verbatim (or
update docs/types if you prefer to remove typed proxy errors).
- Around line 81-94: The fetch path currently allows an empty URL to be
serialized and sent; add a local validation in the same scope where url is
computed (the client.proxy.fetch implementation that uses const url = typeof
input === "string" ? input : String(input)) to reject empty strings—e.g., if url
=== "" or url.trim() === "" then throw an EnkryptifyError with a clear message
(matching request() behavior) before calling
normalizeMethod/normalizeHeaders/parseFetchBody or this.#send; reference the
existing EnkryptifyError type and keep the check near the url computation to
avoid a remote round trip.
🪄 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: d988cca9-af43-42a8-9e3e-722a21713527
📒 Files selected for processing (7)
README.mdsrc/enkryptify.tssrc/errors.tssrc/index.tssrc/proxy.tssrc/types.tstests/proxy.test.ts
| const proxyUrl = config.proxy?.url ?? process.env.ENKRYPTIFY_PROXY_URL ?? DEFAULT_PROXY_URL; | ||
| this.#proxyOnly = config.proxy?.proxyOnly ?? false; | ||
| this.#proxy = new EnkryptifyProxy({ | ||
| proxyUrl, | ||
| auth, | ||
| tokenExchange: this.#tokenExchange, | ||
| workspace: this.#workspace, | ||
| project: this.#project, | ||
| environment: this.#environment, | ||
| usePersonalValues: this.#usePersonalValues, | ||
| logger: this.#logger, | ||
| isDestroyed: () => this.#destroyed, | ||
| }); |
There was a problem hiding this comment.
Avoid a preview deployment as the implicit proxy backend.
The fallback chain ends at https://proxy-poc-black.vercel.app, so any consumer who forgets to set proxy.url / ENKRYPTIFY_PROXY_URL is silently pinned to a non-canonical service. That's a brittle production default and can turn a config mistake into routing proxy auth to the wrong host. Prefer an official stable domain here, or fail fast when no explicit proxy URL is configured.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/enkryptify.ts` around lines 113 - 125, The code uses a brittle fallback
proxy URL (DEFAULT_PROXY_URL / `https://proxy-poc-black.vercel.app`) when
neither `config.proxy?.url` nor `process.env.ENKRYPTIFY_PROXY_URL` is set, which
can silently route traffic to a preview deployment; update the initialization in
`proxyUrl`/`EnkryptifyProxy` use to either require an explicit proxy URL (fail
fast with a clear error when `config.proxy?.url` and `ENKRYPTIFY_PROXY_URL` are
both missing) or replace `DEFAULT_PROXY_URL` with an official stable domain, and
ensure any error message references `proxyUrl`/`config.proxy` so consumers know
to set `ENKRYPTIFY_PROXY_URL` instead of relying on the preview host.
| const url = typeof input === "string" ? input : String(input); | ||
|
|
||
| const method = normalizeMethod(init?.method); | ||
| const headers = normalizeHeaders(init?.headers); | ||
| const body = parseFetchBody(init?.body); | ||
|
|
||
| if ((method === "GET" || method === "HEAD") && body !== undefined) { | ||
| throw new EnkryptifyError( | ||
| `${method} requests cannot include a body. Remove the body or change the method.\n` + | ||
| "Docs: https://docs.enkryptify.com/sdk/proxy", | ||
| ); | ||
| } | ||
|
|
||
| return this.#send( |
There was a problem hiding this comment.
Validate empty URLs in client.proxy.fetch() as well.
request() rejects url: "", but fetch("") currently serializes an empty target URL and lets the proxy fail remotely. That makes the two entry points inconsistent and turns a local argument error into a network round trip.
Suggested fix
- const url = typeof input === "string" ? input : String(input);
+ const url = typeof input === "string" ? input : String(input);
+ if (!url) {
+ throw new EnkryptifyError("Proxy request requires a non-empty `url`.");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = typeof input === "string" ? input : String(input); | |
| const method = normalizeMethod(init?.method); | |
| const headers = normalizeHeaders(init?.headers); | |
| const body = parseFetchBody(init?.body); | |
| if ((method === "GET" || method === "HEAD") && body !== undefined) { | |
| throw new EnkryptifyError( | |
| `${method} requests cannot include a body. Remove the body or change the method.\n` + | |
| "Docs: https://docs.enkryptify.com/sdk/proxy", | |
| ); | |
| } | |
| return this.#send( | |
| const url = typeof input === "string" ? input : String(input); | |
| if (!url) { | |
| throw new EnkryptifyError("Proxy request requires a non-empty `url`."); | |
| } | |
| const method = normalizeMethod(init?.method); | |
| const headers = normalizeHeaders(init?.headers); | |
| const body = parseFetchBody(init?.body); | |
| if ((method === "GET" || method === "HEAD") && body !== undefined) { | |
| throw new EnkryptifyError( | |
| `${method} requests cannot include a body. Remove the body or change the method.\n` + | |
| "Docs: https://docs.enkryptify.com/sdk/proxy", | |
| ); | |
| } | |
| return this.#send( |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/proxy.ts` around lines 81 - 94, The fetch path currently allows an empty
URL to be serialized and sent; add a local validation in the same scope where
url is computed (the client.proxy.fetch implementation that uses const url =
typeof input === "string" ? input : String(input)) to reject empty strings—e.g.,
if url === "" or url.trim() === "" then throw an EnkryptifyError with a clear
message (matching request() behavior) before calling
normalizeMethod/normalizeHeaders/parseFetchBody or this.#send; reference the
existing EnkryptifyError type and keep the check near the url computation to
avoid a remote round trip.
| const response = await fetch(this.#proxyUrl, { | ||
| method: "POST", | ||
| headers: { | ||
| Authorization: `Bearer ${token}`, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(wireBody), | ||
| signal, | ||
| }); | ||
|
|
||
| this.#logger.debug(`Proxy responded with HTTP ${response.status} in ${Date.now() - start}ms`); | ||
|
|
||
| // Return the Response verbatim — whatever status, body, and headers it carries. | ||
| // | ||
| // The Proxy forwards upstream responses unchanged (2xx or not), so mapping status | ||
| // codes to typed errors here is fundamentally unsafe: an upstream 401 from the | ||
| // caller's target API (e.g. OpenWeatherMap rejecting its own API key) is | ||
| // indistinguishable on the wire from a proxy 401 (e.g. Enkryptify token expired), | ||
| // and translating both into AuthenticationError produced wrong, misleading errors. | ||
| // | ||
| // Callers handle non-2xx like native fetch: check `response.ok` / `response.status` | ||
| // and read the body. Proxy-layer errors are delivered as `{error: {code, message}}` | ||
| // JSON bodies that callers can parse for specifics. | ||
| return response; |
There was a problem hiding this comment.
The new proxy error types are unreachable from this implementation.
#send() returns every HTTP response verbatim, and transport failures still bubble out as raw fetch errors. That means the new public ProxyError / ProxyValidationError contract can never actually be observed by callers, while src/index.ts, src/types.ts, and the README now advertise those types. Either add a reliable proxy-origin discriminator and map only those failures, or drop the typed proxy-error contract from this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/proxy.ts` around lines 181 - 204, The `#send` method currently returns
fetch Response verbatim, so callers never observe the advertised
ProxyError/ProxyValidationError types; change `#send` to detect proxy-originated
errors (e.g., check for a proxy-specific response header such as X-Proxy-Error
or the proxy JSON shape {error:{code,message}} on non-2xx responses), parse the
body, and convert those into the exported ProxyError or ProxyValidationError
types before throwing, while letting transport (fetch) exceptions continue to
bubble and leaving non-proxy upstream responses untouched; update the detection
logic inside the `#send` implementation (the method that calls fetch against
this.#proxyUrl and currently returns response) to parse response.json() when the
discriminator is present, construct and throw the correct
ProxyError/ProxyValidationError, and keep other responses returned verbatim (or
update docs/types if you prefer to remove typed proxy errors).
1 participant
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests