Conversation
When the request body stream fails while a file part is being consumed, the parser never learns about it, so it cannot terminate the active file callback. The file's pull loop only checked its `finished` flag and kept pumping an already-failed upstream forever, starving the fiber until an outer timeout fired. Record the upstream cause alongside the existing part-stream exit and fail the active file part with it. Causes that are already a `MultipartError` pass through unchanged, anything else is wrapped as `InternalError`, so `File.content` and `File.contentEffect` keep their declared error type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pVAxS811pzbc8NJtmvCWs
🦋 Changeset detectedLatest commit: 3b560c7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Author
|
The The same job failed identically on a push to Could someone re-run that job when convenient? |
Author
|
Superseded by #8206, which landed the same fix. Closing. |
This branch is waiting to be deployed
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
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.
Fixes #8203
Summary
MultipartErrorsoFile.contentandFile.contentEffectkeep their declared error typecontentEffect, and for a file part that is never consumedRoot cause
In
Multipart.makeChannel,pumprecords an upstream failure inexitand the outer part loop checks it, but the per-file pull loop created inonFileonly checked its ownfinishedflag:The parser is never told about an upstream failure (
parser.end()only runs onDone), so it cannot send the active file callback its terminatingnull.finishedtherefore staysfalseand a consumer drainingpart.contentcallspumpon an already-failed upstream forever. The loop never yields, so the failure surfaces only when an outer watchdog fires, and in the tests here evenEffect.timeoutinside the same fiber never gets a chance to run.This is a different trigger from #7155 / #7156. Those covered parser limit and truncated-input failures, which the parser can observe and which it now terminates by sending
nullto the active file. An upstream stream failure is invisible to the parser, so it has to be handled in the wrapper.Change
makeChannelkeeps the upstream cause in a newupstreamFailureslot next to the existingexit, mapped so a cause that is already aMultipartErrorpasses through and anything else becomesInternalError. The file pull loop drains any buffered chunks first, then ends onfinished, and otherwise fails with that cause rather than pumping again.Because the file channel can now fail, its error type widens from
nevertoMultipartError. This is internal only: the publicFile.contentandFile.contentEffectwere already typed as failing withMultipartError.FileImpl.contentEffecttherefore no longer needs to wrap the channel error, anddefaultWriteFilenow passes an existingMultipartErrorthrough instead of re-wrapping it asInternalError.A file that completed before the failure still ends cleanly, so the parser-limit behavior from #7156 is unchanged.
Validation
pnpm test --run packages/effect/test/unstable/http/Multipart.test.ts(19 tests)main, and pass with the changepnpm test --run packages/effect/test/unstable/http/HttpServerRequest.test.ts packages/effect/test/unstable/http/HttpServer.test.ts packages/effect/test/unstable/http/HttpPlatform.test.tspnpm test --run packages/platform/node/test/MultipartParser.test.ts packages/platform/node/test/HttpApi.test.ts(128 tests)pnpm lint,pnpm checkThe issue reproduces through
@effect/platform-bun, but the hang is in the sharedeffect/unstable/http/Multipartchannel thatBunMultipart.streampipes through, so the regression tests live with the core module. Bun was not available locally to run the reporter's script directly.