feat: add proxy and custom HTTP agent support for Braintree#38
Open
feat: add proxy and custom HTTP agent support for Braintree#38
Conversation
…raintree payment provider
…configuration options for Braintree payment provider
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds configurable HTTP(S) transport for the Braintree plugin: new Changes
Sequence Diagram(s)sequenceDiagram
participant Init as Plugin Init
participant Resolver as createHttpAgent()
participant Custom as customHttpAgent
participant ProxyMod as https-proxy-agent
participant NodeAgent as Node https.Agent
participant Braintree as BraintreeGateway
Init->>Resolver: resolve agent (options)
alt customHttpAgent present
Resolver->>Custom: return provided agent
Resolver-->>Init: agent
else proxyUrl present
Resolver->>ProxyMod: require('https-proxy-agent')
alt require succeeds
ProxyMod-->>Resolver: HttpsProxyAgent(proxyUrl)
Resolver-->>Init: agent
else require fails (module missing)
Resolver--xInit: log warning, fall back
Resolver->>NodeAgent: build from httpAgent or defaults
NodeAgent-->>Resolver: agent
Resolver-->>Init: agent
end
else httpAgent present
Resolver->>NodeAgent: build https.Agent from httpAgent opts
NodeAgent-->>Resolver: agent
Resolver-->>Init: agent
end
Init->>Braintree: new BraintreeGateway({..., customHttpAgent: agent})
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 193-207: The catch block around requiring/constructing
HttpsProxyAgent in the proxy handling for this.options_.proxyUrl should not
treat every error as a missing module; update the try/catch in braintree-base.ts
so it checks the caught error (when requiring 'https-proxy-agent' or calling new
HttpsProxyAgent(this.options_.proxyUrl)) and only fall back to a regular agent
when the error is a module-not-found error (e.g., error.code ===
'MODULE_NOT_FOUND' or equivalent); for all other errors (invalid proxy URL,
construction failures) rethrow or log an error and fail loudly via
this.logger.error/throw so the misconfiguration is not silently ignored—keep
references to require('https-proxy-agent'), new
HttpsProxyAgent(this.options_.proxyUrl), the catch block, and
this.logger.warn/this.logger.error when making the change.
🪄 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: da696554-67d3-4aea-9c97-59b09d3db4d0
📒 Files selected for processing (3)
plugins/braintree-payment/README.mdplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.tsplugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/braintree-payment/README.md (1)
97-98: Document the direct-connect fallback explicitly.The note says to install
https-proxy-agent, but it doesn't mention that the current implementation inplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts:267-297only warns and then continues without the proxy. That behavior is important for locked-down environments because traffic will go out over a different network path than the operator intended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/braintree-payment/README.md` around lines 97 - 98, Update the README to explicitly document the direct-connect fallback: state that when proxyUrl is set but the https-proxy-agent package is not installed, the braintree proxy-agent creation block (the code that inspects proxyUrl/customHttpAgent/httpAgent in braintree-base.ts) logs a warning and continues without a proxy, causing traffic to use a direct connection; mention the security/network implications for locked-down environments and recommend installing https-proxy-agent (or alternately validate and fail fast in the proxy-agent creation code if you want stricter behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 261-318: The createHttpAgent method assumes this.options_.proxyUrl
is a valid URL string and this.options_.httpAgent fields are correctly typed;
update validateOptions (the options validation function) to perform explicit
checks for these new transport options: ensure options_.proxyUrl, if present, is
a non-empty string and a parseable URL; ensure options_.httpAgent, if present,
is an object and its fields (keepAlive boolean, keepAliveMsecs number >=0,
maxSockets/maxFreeSockets numbers >=0, timeout number >=0, rejectUnauthorized
boolean) have the correct types/ranges; on invalid values throw or return a
clear validation error so createHttpAgent can assume well-formed inputs (refer
to createHttpAgent, options_.proxyUrl, and options_.httpAgent to locate the
relevant code).
- Around line 278-289: The current fallback treats any MODULE_NOT_FOUND as
missing https-proxy-agent; narrow the condition in the error handling inside the
BraintreeBase class (around the isModuleNotFound / isRequireMissingProxyAgent
checks) so the fallback only triggers when the error specifically indicates
"Cannot find module 'https-proxy-agent'" (e.g., use the regex check against
error.message or check error.code plus error.message contains
"https-proxy-agent") and remove or tighten the generic MODULE_NOT_FOUND branch;
keep the logger.warn message but only call it when the
https-proxy-agent-specific check matches so other MODULE_NOT_FOUND errors are
allowed to bubble or be rethrown.
---
Nitpick comments:
In `@plugins/braintree-payment/README.md`:
- Around line 97-98: Update the README to explicitly document the direct-connect
fallback: state that when proxyUrl is set but the https-proxy-agent package is
not installed, the braintree proxy-agent creation block (the code that inspects
proxyUrl/customHttpAgent/httpAgent in braintree-base.ts) logs a warning and
continues without a proxy, causing traffic to use a direct connection; mention
the security/network implications for locked-down environments and recommend
installing https-proxy-agent (or alternately validate and fail fast in the
proxy-agent creation code if you want stricter behavior).
🪄 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: 803d36e1-81ff-4fab-9688-45283db7a42b
📒 Files selected for processing (3)
plugins/braintree-payment/README.mdplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.tsplugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 396-402: The validation block that calls new URL(options.proxyUrl)
must not echo the raw options.proxyUrl (which may contain credentials) in the
thrown MedusaError; replace the error message to avoid leaking secrets by either
using a generic message like `Option "proxyUrl" must be a valid URL` or include
only a safe component (e.g., the URL's hostname) after parsing with new
URL(...)—update the try/catch around new URL(options.proxyUrl) in
braintree-base.ts to throw a MedusaError without interpolating options.proxyUrl.
- Around line 427-452: The numeric validation for httpAgent options
(keepAliveMsecs, maxSockets, maxFreeSockets, timeout) must also reject NaN and
non-finite numbers; update each guard in braintree-base.ts (the checks around
keepAliveMsecs, maxSockets, maxFreeSockets, timeout) to use Number.isFinite(...)
(or !Number.isNaN/Number.isFinite) instead of typeof === 'number', e.g. if
(isDefined(keepAliveMsecs) && (!Number.isFinite(keepAliveMsecs) ||
keepAliveMsecs < 0)) throw the existing MedusaError, and apply the same change
for maxSockets, maxFreeSockets and timeout so malformed NaN values are rejected.
🪄 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: 7c620045-e07a-4d35-8c0f-8e09c6080839
📒 Files selected for processing (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (2)
429-455:⚠️ Potential issue | 🟡 MinorReject
NaNin numerichttpAgentfields.The current guards check
typeof value === 'number'andvalue >= 0, butNaNpasses both checks because:
typeof NaN === 'number'istrueNaN < 0isfalse(NaN comparisons always returnfalse)This allows malformed configs to slip through validation.
Proposed fix
Add a helper function and update the guards:
+ const isNonNegativeNumber = (value: unknown): value is number => + typeof value === 'number' && !Number.isNaN(value) && value >= 0; + - if (isDefined(keepAliveMsecs) && (typeof keepAliveMsecs !== 'number' || keepAliveMsecs < 0)) { + if (isDefined(keepAliveMsecs) && !isNonNegativeNumber(keepAliveMsecs)) { throw new MedusaError( MedusaError.Types.INVALID_ARGUMENT, 'Option "httpAgent.keepAliveMsecs" must be a number greater than or equal to 0 in Braintree plugin', ); } - if (isDefined(maxSockets) && (typeof maxSockets !== 'number' || maxSockets < 0)) { + if (isDefined(maxSockets) && !isNonNegativeNumber(maxSockets)) { throw new MedusaError( MedusaError.Types.INVALID_ARGUMENT, 'Option "httpAgent.maxSockets" must be a number greater than or equal to 0 in Braintree plugin', ); } - if (isDefined(maxFreeSockets) && (typeof maxFreeSockets !== 'number' || maxFreeSockets < 0)) { + if (isDefined(maxFreeSockets) && !isNonNegativeNumber(maxFreeSockets)) { throw new MedusaError( MedusaError.Types.INVALID_ARGUMENT, 'Option "httpAgent.maxFreeSockets" must be a number greater than or equal to 0 in Braintree plugin', ); } - if (isDefined(timeout) && (typeof timeout !== 'number' || timeout < 0)) { + if (isDefined(timeout) && !isNonNegativeNumber(timeout)) { throw new MedusaError( MedusaError.Types.INVALID_ARGUMENT, 'Option "httpAgent.timeout" must be a number greater than or equal to 0 in Braintree plugin', ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts` around lines 429 - 455, The numeric validation for httpAgent fields (keepAliveMsecs, maxSockets, maxFreeSockets, timeout) currently allows NaN because it only checks typeof === 'number' and >= 0; update those guards to reject NaN/Infinity by using Number.isFinite(value) (or a small helper like isValidNonNegativeNumber) and value >= 0, throwing the same MedusaError for each variable (keepAliveMsecs, maxSockets, maxFreeSockets, timeout) in braintree-base.ts so malformed configs are rejected.
398-405:⚠️ Potential issue | 🟠 MajorDo not echo the raw
proxyUrlin the validation error.The proxy URL can legally contain credentials in the format
http://username:password@proxy.example.com:8080. Interpolatingoptions.proxyUrlin the error message will leak these credentials into exception logs and telemetry when the config is invalid.Proposed fix
try { new URL(options.proxyUrl); } catch { throw new MedusaError( MedusaError.Types.INVALID_ARGUMENT, - `Option "proxyUrl" must be a valid URL in Braintree plugin: "${options.proxyUrl}"`, + 'Option "proxyUrl" must be a valid URL in Braintree plugin', ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts` around lines 398 - 405, The validation error exposes sensitive credentials by interpolating options.proxyUrl into the thrown MedusaError; change the error message in the proxy URL validation (the try/catch around new URL(options.proxyUrl)) to avoid echoing the raw value—either omit the URL entirely or log a sanitized form (e.g., only the hostname/port obtained from new URL(...) or a generic message like "invalid proxy URL") when throwing MedusaError.Types.INVALID_ARGUMENT for proxyUrl validation so credentials are not leaked.
🧹 Nitpick comments (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
48-49: Unusedhttpimport.The
httpmodule is imported but never used. Onlyhttps.Agentis instantiated increateHttpAgent(). Consider removing the unused import.-import http from 'http'; import https from 'https';The return type
http.Agent | https.Agent | undefinedis still valid sincehttps.Agentextendshttp.Agent, but the explicit import is unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts` around lines 48 - 49, Remove the unused "http" import and update the agent return type to use only https.Agent (or the broader Agent type from https) in createHttpAgent; specifically, delete the line importing http, change the function signature/return annotation in createHttpAgent() from "http.Agent | https.Agent | undefined" to "https.Agent | undefined" (or simply "https.Agent" union with undefined), and ensure any other references to http.Agent in this file are updated to https.Agent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 429-455: The numeric validation for httpAgent fields
(keepAliveMsecs, maxSockets, maxFreeSockets, timeout) currently allows NaN
because it only checks typeof === 'number' and >= 0; update those guards to
reject NaN/Infinity by using Number.isFinite(value) (or a small helper like
isValidNonNegativeNumber) and value >= 0, throwing the same MedusaError for each
variable (keepAliveMsecs, maxSockets, maxFreeSockets, timeout) in
braintree-base.ts so malformed configs are rejected.
- Around line 398-405: The validation error exposes sensitive credentials by
interpolating options.proxyUrl into the thrown MedusaError; change the error
message in the proxy URL validation (the try/catch around new
URL(options.proxyUrl)) to avoid echoing the raw value—either omit the URL
entirely or log a sanitized form (e.g., only the hostname/port obtained from new
URL(...) or a generic message like "invalid proxy URL") when throwing
MedusaError.Types.INVALID_ARGUMENT for proxyUrl validation so credentials are
not leaked.
---
Nitpick comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 48-49: Remove the unused "http" import and update the agent return
type to use only https.Agent (or the broader Agent type from https) in
createHttpAgent; specifically, delete the line importing http, change the
function signature/return annotation in createHttpAgent() from "http.Agent |
https.Agent | undefined" to "https.Agent | undefined" (or simply "https.Agent"
union with undefined), and ensure any other references to http.Agent in this
file are updated to https.Agent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d4efcb6-58de-43b2-b7eb-ba6d597ad8e9
📒 Files selected for processing (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
…core/braintree-base.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ensure non-negative values
…/medusa-plugins into chore/proxy-support
…pAgent checks in Braintree plugin
dwene
reviewed
Apr 21, 2026
| } | ||
|
|
||
| private createHttpAgent(): http.Agent | https.Agent | undefined { | ||
| // Backward compatibility: if customHttpAgent is directly provided, use it |
Contributor
There was a problem hiding this comment.
backwards compatibility? We didn't have this before right? Who would be providing options.customHttpAgent?
dwene
approved these changes
Apr 21, 2026
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.
Summary
customHttpAgent,proxyUrl-based proxy agent, and configurable standardhttpAgent.Motivation
Some deployments require egress through corporate proxies or custom TLS/socket behavior. This change makes the provider usable in restricted network environments without forking provider code.
What’s Included
customHttpAgent: pass a prebuilt Node HTTP(S) agent.proxyUrl: build an HTTPS proxy agent from URL (e.g. authenticated proxy).httpAgent: define standard HTTPS agent settings (keepAlive, socket limits, timeout, TLS validation, etc.).customHttpAgent->proxyUrl->httpAgent.customHttpAgent.HttpAgentConfigand the new options.Behavioral Notes
proxyUrlis configured, installhttps-proxy-agentin the host project.customHttpAgentis intended for advanced/custom transport scenarios and overrides all other agent options.Summary by CodeRabbit
New Features
Documentation
Chores