Skip to content

fix: validate per-request transport options - #422

Merged
JoshMock merged 2 commits into
elastic:mainfrom
skdas20:fix/per-request-option-validation
Nov 13, 2026
Merged

JoshMock merged 2 commits into
elastic:mainfrom
skdas20:fix/per-request-option-validation

Conversation

@skdas20

@skdas20 skdas20 commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Per-request transport overrides currently bypass the constructor’s configuration checks. A negative retry count can skip the request loop, while response-size overrides can exceed Node’s supported buffer limits.

Use the same validation for constructor options and per-request overrides of maxRetries, maxResponseSize, and maxCompressedResponseSize. Also enforce the stated nonnegative-integer contract for retry counts in both paths, rejecting strings, fractional values, NaN, and Infinity with ConfigurationError.

Fixes #405.

Validation: npm test (CommonJS/ESM builds, lint, unit and acceptance tests). The new regression test exercises eight invalid configurations through both entry points; it produces 13 failed assertions on unchanged main. Existing valid retry and response-size override tests remain covered by the full suite.

@cla-checker-service

cla-checker-service Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

💚 CLA has been signed

@JoshMock JoshMock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. You'll need to sign the CLA before we can merge this PR, though. If that isn't signed, we'll close this unmerged soon.

@skdas20

skdas20 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

LGTM. You'll need to sign the CLA before we can merge this PR, though. If that isn't signed, we'll close this unmerged soon.

Signed the CLA

@JoshMock
JoshMock enabled auto-merge (squash) November 2, 2026 18:34
@skdas20

skdas20 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @JoshMock, thanks again for the review! The CLA is signed and all checks pass, so this should be ready whenever you have a moment to merge. Happy to rebase onto main if needed.

@JoshMock
JoshMock disabled auto-merge November 13, 2026 16:08
@JoshMock
JoshMock enabled auto-merge (squash) November 13, 2026 16:08
@JoshMock
JoshMock merged commit 4426a2a into elastic:main Nov 13, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transport options are validated in the constructor, but validation is bypassed when the same options are overridden on a per-request basis

2 participants