Commit Graph

4 Commits

Author SHA1 Message Date
Marco
55b6ca6573 fix(compression): fall back in-process when the compression worker fa… (#13637)
* fix(compression): fall back in-process when the compression worker fails (#13145)

The worker pool resolved every worker fault with the *uncompressed* body instead
of reporting it. `PendingJob` had no reject path at all, so a thread error, a
worker exit, a dispatch timeout, or an engine error posted back as
`type: "error"` all resolved as `{ compressed: false, stats: null }`.

`applyCompressionAsync` then treated that as a legitimate "nothing to compress"
result and returned it as-is, so the request reached the provider uncompressed
while the response header still announced the selected plan ("stacked") — the
header is emitted before the pipeline runs. Nothing was logged at any level, and
`compression_analytics` stayed empty because rows are only written when a
compressed result is reported. The net effect was compression silently disabled
for every worker-eligible request.

The worker is a throughput optimisation, not a behavioural variant, so a worker
fault must degrade to the in-process pipeline rather than to no compression:

- `PendingJob` gains `reject`; `fail()` delegates to a new `abort()` that clears
  the slot timeout and rejects with a diagnostic cause (thread error, exit code,
  or timeout budget).
- An `error` message from the worker is propagated instead of being swallowed.
- `applyCompressionAsync` catches the rejection and falls through to the
  in-process path, logging the cause. The logger is imported lazily and
  defensively: `compressionWorker.ts` imports this module, so a static import
  would pull the logger into the worker bundle, and a logging failure must never
  be able to break compression itself.

`close()` keeps resolving with the unchanged body — shutdown is not a fault.

The regression test drives a real worker fault via
`OMNI_COMPRESSION_WORKER_TIMEOUT_MS` rather than mocking the module, since this
project's tsx/ESM + node:test setup has no `mock.module()` support. Its options
are fully populated on purpose: `runCompressionAsync` forwards them into
`workerOptions`, and `isStrictlySerializable` rejects an object holding
`undefined` values — which would route the test through the in-process path and
assert nothing. Production requests always carry all of those fields, which is
why the worker path is taken there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGJQT3E6iJZq4zNGwfjkPs

* fix(compression): keep the timeout path uncompressed, retry only fast worker faults (#13145)

Review follow-up: the in-process fallthrough ran the full pipeline on the main
event loop for *every* worker fault, including a dispatch timeout. A timeout
means the worker already spent its whole budget on that body, so re-running the
same CPU-bound work inline would stall other in-flight requests — strictly worse
than not compressing on a shared gateway.

Faults are now typed by whether recovery is cheap:

- `CompressionWorkerError.retryInProcess` distinguishes fast faults (thread
  error, worker exit, engine throw — no work was done, so the in-process path
  costs what the worker would have) from a dispatch timeout.
- Timeouts keep the original degrade-to-uncompressed behaviour, but are now
  reported. The defect this PR fixes is the silent swallow, not the degrade.

Also strips `reject` from the structured-clone wire job. It is a function, so
leaving it on the object handed to `postMessage` threw `DataCloneError` before
the worker ever saw the job — turning every dispatch into an immediate fault.

Adds the missing `changelog.d/fixes/` fragment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(compression): narrow the worker thread-error type for typecheck:core

@types/node 26 types the Worker "error" event payload as unknown, not
Error, so `error?.message` failed typecheck:core (TS2339). Narrow with
an instanceof check before reading .message.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: marcs7 <marcs7@users.noreply.github.com>
2026-09-18 13:13:25 -03:00
Koosha Paridehpour
7a0b0c64fb fix(compression): track only the recursion path in isStrictlySerializable (#13154) (#13423)
* fix(compression): remove isStrictlySerializable gate that rejects valid bodies (#13154)

Fixes #13154

isCompressionWorkerEligible used isStrictlySerializable to pre-validate
bodies before posting them to a worker thread. The gate is stricter than
structuredClone (which postMessage uses natively), rejecting:
- undefined values (common in optional config fields)
- Date, Map, Set, Uint8Array, RegExp (all structuredClone-compatible)
- Shared (non-cyclic) sub-objects (misread as cycles)

This caused compression to fall back to inline execution on the main
event loop, blocking every concurrent request for the duration of
compression passes — the exact failure mode of #10300.

The serializability walk is a slower, buggier duplicate of the check
postMessage already performs. Removing it:
- Eliminates a recursive walk of the entire body on the main thread
- Fixes false rejections that prevent worker offload
- Allows the catch block at the call site to properly fall through
  to inline compression instead of silently shipping uncompressed

* fix(compression): track only the recursion path, not the whole tree, in isStrictlySerializable (#13154)

Restores the cycle-detection gate instead of removing it: the original
bug was a single `seen` set shared across the entire recursion tree,
never backtracked, so two sibling branches referencing the SAME
non-cyclic sub-object were misread as a cycle. Adding to `seen` before
descending and removing it after (try/finally) fixes the false positive
while a genuine cycle is still rejected before it ever reaches
postMessage/the worker.

Also reverts the worker-failure catch in runCompressionAsync back to
returning the body uncompressed: a worker timeout means the compression
was already too heavy for the worker's own budget, so falling through
to run that same heavy compression synchronously on the main event loop
defeats the point of offloading it to a worker in the first place.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: Koosha Pari <koosha@phenotype.ai>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
2026-09-17 12:34:09 -03:00
Diego Rodrigues de Sa e Souza
3f62e43696 test(compression): assert idle eviction terminates at the resource level (#13371)
Merged as the credit vehicle for #12542. Reverse-TDD verified on the tip: 8/8 with the fix, 7/8 with `terminate()` disabled.
2026-09-11 17:46:22 -03:00
Markus Hartung
04dba0460e fix(responses-continuation): recover a real id/output for passthrough and translate-mode replies (#11434)
Retargetado para release/v3.8.51 (release/v3.8.50 está congelada — freeze issue #11439). Validado em lote combinado (batch-0824h2, junto de #11435/#11436/#11437) contra o tip de release/v3.8.51: typecheck:core limpo, gates estáticos OK, 127/127 testes focados passando.

Investigação sólida com repro real via container isolado, três causas independentes identificadas e corrigidas com testes de regressão dedicados para cada uma. Obrigado pela contribuição!
2026-08-24 19:57:12 -03:00