Skip to content

fix(request-executor): serialize every request option in toJSON - #770

Draft
aborovsky wants to merge 1 commit into
nextfrom
fix/request-tojson-complete
Draft

fix(request-executor): serialize every request option in toJSON#770
aborovsky wants to merge 1 commit into
nextfrom
fix/request-tojson-complete

Conversation

@aborovsky

@aborovsky aborovsky commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #769, addressing #769 (comment).

Problem

Request.toJSON() omitted timeout, maxContentSize, decompress, keepAlive and encoding. transformScript rebuilds the request from that projection, so every script-transformed request was reconstructed from a lossy copy.

Round-tripping a request through toJSON() before this change:

field before after round-trip
timeout 1234 undefined
maxContentSize 99 undefined
keepAlive true undefined
decompress false true
encoding 'base64' undefined (fixed in #769)

Impact, in order of severity:

  • timeout and maxContentSize are reachable and were genuinely lost. RepeaterServerRequestEvent carries both from the Bright server and ServerRepeaterLauncher spreads the event straight into new Request({ ...event }). After a script transform they were undefined, so applyCurlTimeout silently fell back to the executor-level timeout and the response cap fell back to maxContentLength.
  • decompress was worse than dropped. The constructor defaults it to true, so omitting it flipped an explicit false. Nothing in the codebase currently sets decompress: false, so this was not reachable in practice.
  • keepAlive is unread. Nothing consumes Request.keepAlive; connection reuse is driven entirely by the executor-level reuseConnection. Included for completeness only.

Change

toJSON() now serializes the full option set the constructor accepts, so new Request(request.toJSON()) is a faithful copy.

Because toJSON() now carries encoding, the script payload explicitly overrides it to undefined. The body handed to a script is already decoded, so reporting encoding: 'base64' next to it would describe it incorrectly and mislead any script that inspects options.encoding. This keeps the decoded-view contract intact and leaves the restore logic added in #769 behaving exactly as it does today.

Tests

Four new tests, each verified to fail without the source change:

  • toJSON round-trips every option the constructor accepts
  • an explicit decompress: false survives instead of defaulting back to true
  • a per-request timeout is honoured after a script transform (without the fix the request is unbounded and the slow response arrives successfully)
  • the script receives timeout / maxContentSize / decompress but no encoding

src/ suite: 264 passed across 24 suites. src/RequestExecutor/: 98 passed (94 on next + 4 new). eslint clean.

tsc --noEmit reports one error at HttpRequestExecutor.spec.ts:158; it is pre-existing on next and untouched by this branch (the only hunk here starts at line 1495).

E2E and smoke suites were not run — they require live Bright API credentials.

@aborovsky aborovsky self-assigned this Aug 6, 2026
@aborovsky aborovsky added the Type: bug Something isn't working. label Aug 6, 2026
`Request.toJSON()` omitted `timeout`, `maxContentSize`, `decompress`,
`keepAlive` and `encoding`, and `transformScript` rebuilds the request from it.
Every script-transformed request therefore lost its per-request timeout and
response size cap and silently fell back to the executor-level defaults, both
of which the Bright server sets per request. `decompress` was worse than
dropped: the constructor defaults it to `true`, so an explicit `false` was
flipped.

Serialize the full option set instead. The script payload still reports no
`encoding`, because the body handed to the script is already decoded and
claiming otherwise would describe it incorrectly.
@aborovsky
aborovsky force-pushed the fix/request-tojson-complete branch from dee3c5f to 0a8b54e Compare August 6, 2026 11:14
@aborovsky aborovsky changed the title fix(repeater): preserve body bytes when script re-declares encoding fix(request-executor): serialize every request option in toJSON Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: bug Something isn't working.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant