Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression: Errors not emitted in streams #14314

Merged
merged 1 commit into from
Nov 27, 2018

Conversation

voxpelli
Copy link
Contributor

Regression introduced in #14182 resulted in errors no longer being emitted on streams, breaking consumers.

The cause is a faulty implementation of the destroy() method of ReactMarkupReadableStream, which resulted in no error event being thrown even though one expected it.

This patch fixes that + adds tests for the fixes.

Regression introduced in facebook#14182 resulted in errors no longer being emitted on streams, breaking many consumers.

Co-authored-by: Elliot Jalgard <elliot.j@live.se>
Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

I'm not deeply familiar with Node Readable API but base implementation is function (err, cb) { this.push(null); cb(err); }' so this looks good.

@gaearon gaearon merged commit ee3ef3a into facebook:master Nov 27, 2018
@voxpelli voxpelli deleted the fix-stream-error-emitting branch November 27, 2018 16:47
@greaveselliott
Copy link

Hi, when will this change be released?

@gaearon
Copy link
Collaborator

gaearon commented Dec 6, 2018

If you need it urgently you can build React from master and get the fix. I can't give you an exact day — we generally release things when we feel they're stable, and this means giving some time for other changes to shake out. Probably within a few weeks.

@greaveselliott
Copy link

greaveselliott commented Dec 6, 2018 via email

@decompil3d
Copy link

Fix appears to be in 16.7.0

jetoneza pushed a commit to jetoneza/react that referenced this pull request Jan 23, 2019
Regression introduced in facebook#14182 resulted in errors no longer being emitted on streams, breaking many consumers.

Co-authored-by: Elliot Jalgard <elliot.j@live.se>
n8schloss pushed a commit to n8schloss/react that referenced this pull request Jan 31, 2019
Regression introduced in facebook#14182 resulted in errors no longer being emitted on streams, breaking many consumers.

Co-authored-by: Elliot Jalgard <elliot.j@live.se>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants