Skip to content

Fix broken test on linux#2699

Merged
mcollina merged 5 commits into
mainfrom
fix-broken-test-on-linux
Feb 5, 2024
Merged

Fix broken test on linux#2699
mcollina merged 5 commits into
mainfrom
fix-broken-test-on-linux

Conversation

@mcollina

@mcollina mcollina commented Feb 5, 2024

Copy link
Copy Markdown
Member

Fixes #2697

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Comment thread lib/fetch/index.js
fetchParams.controller.controller.enqueue(buffer)
try {
fetchParams.controller.controller.enqueue(buffer)
} catch {}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@KhafraDev please review this. A wpt is failing without it: https://github.com/nodejs/undici/blob/main/test/wpt/tests/fetch/api/basic/stream-safe-creation.any.js

Uncaught exception in "D:\a\undici\undici\test\wpt\tests\fetch\api\basic\stream-safe-creation.any.js":
TypeError [ERR_INVALID_STATE]: Invalid state: ReadableStream is already closed
    at ReadableByteStreamController.enqueue (node:internal/webstreams/readablestream:1160:13)
    at fetchParams.controller.resume (D:\a\undici\undici\lib\fetch\index.js:2024:43)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================
Uncaught exception in "D:\a\undici\undici\test\wpt\tests\fetch\api\basic\stream-safe-creation.any.js":
TypeError [ERR_INVALID_STATE]: Invalid state: ReadableStream is already closed
    at ReadableByteStreamController.enqueue (node:internal/webstreams/readablestream:1160:13)
    at fetchParams.controller.resume (D:\a\undici\undici\lib\fetch\index.js:2024:43)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
================================================================================================

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina

mcollina commented Feb 5, 2024

Copy link
Copy Markdown
Member Author

@KhafraDev please take a look at the changes in fetch(), I'm not 100% convinced but the WPTs and our test pass.

Signed-off-by: Matteo Collina <hello@matteocollina.com>

@KhafraDev KhafraDev 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.

I have 0 time this week to dig into it, but nothing seems off to me.

@Uzlopak

Uzlopak commented Feb 5, 2024

Copy link
Copy Markdown
Contributor

I will have a look now

@mcollina mcollina merged commit cf4d90d into main Feb 5, 2024
@mcollina mcollina deleted the fix-broken-test-on-linux branch February 5, 2024 23:31
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.

Security Update broke test/fetch/pull-dont-push.js?

4 participants