Skip to content

fix(fetch): preserve error code in decompression pipeline for retry logic#40946

Merged
pavelfeldman merged 2 commits into
microsoft:mainfrom
SebTardif:fix-fetch-retry-compressed
May 28, 2026
Merged

fix(fetch): preserve error code in decompression pipeline for retry logic#40946
pavelfeldman merged 2 commits into
microsoft:mainfrom
SebTardif:fix-fetch-retry-compressed

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

What this PR does

Fixes maxRetries being silently broken for compressed HTTP responses.

In packages/playwright-core/src/server/fetch.ts, the decompression pipeline error callback wraps all errors in new Error(...), destroying the .code property. The retry logic in _sendRequestWithRetries checks e.code !== 'ECONNRESET' to decide whether to retry. Since the wrapped error has no .code, retries never happen for compressed responses (the vast majority of real-world HTTP traffic, since accept-encoding: gzip,deflate,br is sent by default).

Fix: Added isNetworkConnectionError() helper that checks for ECONNRESET, EPIPE, ECONNABORTED. Network errors are passed through unwrapped to preserve .code; only actual decompression failures get wrapped.

AI Disclosure

Developed with AI assistance (Grok by xAI) in a human-in-the-loop workflow. All code reviewed and verified by the human author.

@dgozman dgozman requested a review from yury-s May 22, 2026 09:49
Copy link
Copy Markdown
Member

@yury-s yury-s left a comment

Choose a reason for hiding this comment

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

The change looks good, but please add a test.

…ogic

When a compressed HTTP response (gzip/br/deflate) encounters a network
error like ECONNRESET during body streaming, the pipeline error callback
and body error handler were wrapping the original error in a new Error,
destroying the .code property. The retry logic in _sendRequestWithRetries
checks e.code !== 'ECONNRESET' to decide whether to retry, so retries
never happened for compressed responses.

Fix: check if the error is a network error (ECONNRESET, EPIPE,
ECONNABORTED) and pass it through unwrapped so the retry logic can see
the code. Only wrap non-network errors as decompression failures.
@SebTardif SebTardif force-pushed the fix-fetch-retry-compressed branch from ec419c9 to 7de5f9e Compare May 22, 2026 21:21
@Skn0tt Skn0tt requested a review from yury-s May 27, 2026 09:21
@github-actions

This comment has been minimized.

Move the response 'aborted' handler to the non-compressed branch only.
For compressed responses, the decompression pipeline callback now handles
connection errors, preserving .code for the retry logic.

Add test that sends gzip response headers then destroys the socket
mid-stream, exercising the pipeline error path that was previously
unreachable due to 'aborted' intercepting first.
@github-actions

This comment has been minimized.

@pavelfeldman pavelfeldman merged commit dfe5ab5 into microsoft:main May 28, 2026
43 of 45 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [msedge] › mcp/annotate.spec.ts:137 › should abort MCP annotation when last screenshot is removed @mcp-windows-latest-msedge
❌ [webkit] › mcp/cli-killall.spec.ts:42 › kill-all kills filtered dashboard pid @mcp-windows-latest-webkit

7179 passed, 1113 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

3 flaky ⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:647 › screencast › should capture full viewport `@chromium-ubuntu-22.04-node22`
⚠️ [webkit-library] › library/beforeunload.spec.ts:20 › should close browser with beforeunload page `@webkit-ubuntu-22.04-node20`

43938 passed, 862 skipped


Merge workflow run.

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.

3 participants