stream: finish must always follow error #13850

mcollina · 2017-06-21T18:11:26Z

When _write completes with an Error, 'finish' was emitted before 'error' if the callback was asynchronous. This commit restore the previous behavior.
The logic is still less then ideal, because we call the write() callback before emitting error if asynchronous, but after if synchronous. This commit do not try to change the behavior.

Fixes: #13812

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

stream

When _write completes with an Error, 'finish' was emitted before 'error' if the callback was asynchronous. This commit restore the previous behavior. The logic is still less then ideal, because we call the write() callback before emitting error if asynchronous, but after if synchronous. This commit do not try to change the behavior. This commit fixes a regression introduced by: nodejs#13195. Fixes: nodejs#13812

mcollina · 2017-06-21T18:18:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/8782/

calvinmetcalf · 2017-06-21T18:34:59Z

does this call flush if there is an error ?

phated · 2017-06-21T19:24:02Z

This fixes our test failures for vinyl-fs. How long will this take to get upstreamed? It's blocking a major release

mcollina · 2017-06-21T19:44:21Z

It behave the exact same as before, I did not touch that logic. The actual answe is.. it depends.

calvinmetcalf

lgtm

mcollina · 2017-06-21T20:19:39Z

Tests are ok.

mcollina · 2017-06-21T20:36:09Z

@nodejs/ctc may I land this before the 48 hours, so that we can get it into a patch release asap?

addaleax · 2017-06-21T20:46:03Z

I am okay with fast-tracking. (I’ve chatted with @mcollina a bit and we agreed it would make sense to target a 8.1.3 patch release for next week for multiple reasons, I’m currently working with putting that together.)

nodejs/node#13850

mcollina · 2017-06-21T21:26:25Z

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/888/

nodejs/node#13850

jasnell · 2017-06-22T15:47:58Z

Definitely +1 to fast-tracking

cjihrig · 2017-06-22T15:51:17Z

I'm fine with fast tracking it. But, if the release isn't going out until next week, what is the rush? The normal period will have elapsed by then.

mcollina · 2017-06-22T15:53:22Z

I'm actually fine either way. I'm going to be off until Monday, so I will not be able to do the merge.
If any of you do it tomorrow, we are ok. Or I can do it today.

mcollina · 2017-06-22T22:39:02Z

Landed as b443288.

When _write completes with an Error, 'finish' was emitted before 'error' if the callback was asynchronous. This commit restore the previous behavior. The logic is still less then ideal, because we call the write() callback before emitting error if asynchronous, but after if synchronous. This commit do not try to change the behavior. This commit fixes a regression introduced by: #13195. Fixes: #13812 PR-URL: #13850 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Notable changes * **Stream** Two regressions with the `stream` module have been fixed: * The `finish` event will now always be emitted after the `error` event if one is emitted: [[`0a9e96e86c`](0a9e96e86c)] [#13850](#13850) * In object mode, readable streams can now use `undefined` again. [[`5840138e70`](5840138e70)] [#13760](#13760)

MylesBorins · 2017-07-17T18:45:06Z

Is this applicable to lts?

mcollina · 2017-07-18T08:36:14Z

At the moment no. Only if you backport #13195 first. Also, we had a bit of regressions on this one, so we might want a) not to backport them at all or b) wait that everything stabilizes.

@mcollina

* perf fix for Duplex require replacement I was noticing that RegExps called from Module._resolveLookupPaths were getting to be the hottest symbol in some profiles. readable-stream seems to be requiring Duplex from inside functions. It would be faster to not do a require on each invocation of Readable, ReadableState, Writable, WritableState. * build based on v5.1.0 * bump process-nextick-args * 2.0.5 * fix node repo link * bump isarray to 1.x juliangruber/isarray@0.0.1...v1.0.0 * Update package.json desc.: iojs v2.x => Node.js * Make general updates to build script. * Make processFile() accept URL or local path. * Update Node version in README on build. * Define RegExp's up top. * build for 5.5.0, fix build scripts to deal with arrow functions amongst other things and browser build tweaks * initial build for v5.6.0 * undo the let usage * make sure const and let replacements are not in the middle of words * build based on 5.7.0 now with babel new build uses babel for arrow functions and const/let stuff also makes iThings allowed failures * remove "retain lines" * fix for sync write issues in older node * build for 5.8.0 * bump tape version (mainly to get the tests to rerun) * 2.0.6 * build for 5.9.1 * build for 5.9.1 * and fix build * test against microsoft edge (#194) * add passthrough env variable to allow testing of native streams, also adds coverage * 2.1.0 * fixes #205 reuse existing import of "stream" * 2.1.1 * fix browserify build * 2.1.2 * build for v6.1.0 plus fixes for assert and buffer changes * 2.1.3 * update npm ignore * try ngrok * 2.1.4 * re-enable iThing tests and update zuul * add node 6 * add back ngrok * reenable iThings * maybe older android? * nope minimum android is 3.0 * maybe older ie? * no older ie, maybe older iThings * nope * 6.2.1 (#218) build for 6.2.1 * rm unneeded test * deal with template strings * rewrite test for older node * remove ngrok * trailing comma, I'm done for the day * add sauce connect * patch for wonky setTimeout in older node * fix typo * rm sauce connect * Revert "rm sauce connect" This reverts commit c213f1e. * rm older ithing * build for 6.2.2 * add shorthand properties babel transform * fix for test that has for-of loop * iThings seem to be broken, giving up * back to local tunnel * and remove from .zuul * first stab at 6.3.1 * bufferlist * fix prepend * fix tests more * 2.1.5 * build based on 6.4.0 * Added @mcollina to the README. (#236) * first stab at 6.5.0 * rm unneded regex * inline isArray * build for 7.0.0 * bundle dependencies * build for 7.1.0 and remove documentation but include link to node docs * 2.2.0 * this is why we can't have nice things, fixes (yarnpkg/yarn#1774) * 2.2.1 * fix gulp weirdness * 2.2.2 * Added the governance template. * Updated after @williamkapke suggestions. * build for 7.3.0 (#251) * Exclude `stream` module in react-native (#258) * 2.2.3 * node v7.7.2 build (#262) * replace bind() * re-add WriteReq and CorkedRequest constructors * Updated to v7.7.2, refactored the build to not crawl The new build system uses the tarball from nodejs.org. * try updating zuul * just remove android * 2.2.4 * removed duplicate writereq. See nodejs/readable-stream@7af74b0#commitcomment-21310482 * Bumped v2.2.5. * Fully revert #255 as not released yet in core. * Added node v7 to .travis.yml * Updated to v7.7.3. * Bumped v2.2.6. * remove bizzare circular deps * 2 input files * fix typo * Maintain the same interface when using READABLE_STREAM=disable * Added -browser.js files for Duplex and Writable. * Bumped string_decoder to v1.0.0. * Avoid caret during installation This restores support for older versions of NPM. * Bumped v2.2.7 * Pull node 7.8.0. test passes w/ READABLE_STREAM=disable. * Bumped v2.2.8 * switch to simpler method of stream substitution * and build * use a browser sub for the streams vs event emitter switch * move the stream and stream-browser to somewhere better * remove react-native field * Bumped v2.2.9. * README: link to specific api docs about streams (#279) * use safe-buffer (#282) use safe-buffer * Updated to node v7.10.0. * Better .travis.yml * Disabled test/parallel/test-stream-unpipe-event.js in node 0.8. * Added node 8 to .travis.yml * Bumped v2.2.10. * Depend with ~ over SafeBuffer. * Test that we do not depend using ^, to support old npm. * Bumped v2.2.11. * updated package inherits from 2.0.1 to 2.0.3 (#291) * Build for node 8.1.0 * re-add WriteReq and CorkedRequest constructors * 2.3.0 * READABLE_STREAM=disable works also with writable.js * Import from Node 8.1.2. * Bumped v2.3.1 * Convert classes loosely * Forward-port of nodejs#13850 nodejs/node#13850 * Bumped v2.3.2. * Partially solves perf issues with the UInt8Array checks. Fixes nodejs/readable-stream#302 * Use instanceof check for Uint8Array. * Removed useless typeof check. * Updated from Node 8.1.3. * Bumped v2.3.3 * add lrlna to readme * built 8.9.4. * Working on old nodes * Updated README * Updated dependencies * Fix tests for Node master / citgm * Revert "Fix tests for Node master / citgm" This reverts commit 3dc883c734c7990f803ac05a8a64ce70d0815b1d. * Proper fix, soft-detecting there is a custom inspect. * removed saucelabs testing for now * Fix browser support for util.inspect.custom fix * Bumped v2.3.4 * Support for the Foundation ci. * Fixed broken lolex integration. * Bumped v2.3.5 * Fix lolex integration test on old nodes * Removed zuul as a dependency. * Bump string_decoder to versio 1.1.1 (#330) * Don't add the Array.prototype.forEach replacement to files that don't use forEach. (#331) * Updated to Node 8.11.1 (#332) * Bumped v2.3.6. * Built with 29 tests failing * 27 tests to go * Down do 20 failing tests * removed comment * 18 tests to go. * .npmignore: "docs/" => "doc/" to match real name (#342) * Updated to Node 10.5.0. 2 failures left to go. * all tests passing * Node.js 6 working. * Removed dependency on core-util-is * removed dependency on safe-buffer * only a single entry point * Integrate airtap (#345) * add airtap for continuous browser tests * ad-hoc fixes for browser tests * comment out failing browser tests * restore safari * Updated. * more airtap quirks * remove --no-coverage for airtap * better sauce credentials for travis * Maybe better env variables for airtap * removed sauce credentials from .travis.yml * added back all browsers * added back loopback host * Updated README links for sauce and travis * Suport for IE11 * README tweaks * Bumped v3.0.0-rc.1 * Removed babel-transform-runtime. Updated to Node 10.6.0. * Added back IE 11. * Bumped v3.0.0-rc.2 * Updated to Node 10.8.0. * Bumped v3.0.0-rc.3 * browsers tests should be passing again * Added a thanks note to Sauce Labs * Added usage section. * Removed inactive members (#351) * Added changelog of breaking changes * Bumped v3.0.0 Fixes #283 Fixes #284 Fixes #212 Fixes #297 Fixes #346 Fixes #339 Fixed #335 * Fix error messages (#352) * fix error messages to match node 10.8.0 * npm run update-browser-errors * test errors * Removed process-nexttick-args as it's not needed anymore (#354) * Removed process-nexttick-args as it's not used anymore * Rmoeved process-nexttick-args everywhere, it's not needed * Bumped v3.0.1. * Import Node 10.9.0. Redo the fix for util.inspect.custom (#357) * Import Node 10.9.0. Redo the fix for util.inspect.custom Fixes nodejs/readable-stream#355 * Removed util.inspect and util.inspect.custom poly in the browser test * fallback to 'inspect' if util.inspect.custom is not available * Bumped v3.0.2. * remove Buffer constructor usage (#358) (#359) * remove Buffer constructor usage (#358) * Replace occurrences of `new Buffer(...)` with `Buffer.alloc(...)` or `Buffer.from(...)`, as appropriate * squash! remove unused block * replace Buffer.alloc() with Buffer.from() where appropriate (#360) * Add node >= 6 to package.json (#362) * Updated to Node 10.10 (#363) * Updated to Node 10.10 * fix failing test, the code is correct * Bumped v3.0.3. * Update to node 10 11 (#366) * Fix errors in IE11 * Polyfill Number.isNaN * Don't create Symbol.asyncIterator if Symbol is not defined Fixes #364 * Updated to Node 10.11.0. * Fixed browsers tests * Bumped v3.0.4. * Emit no experimental warning in browser. (#367) closes #361 * Bumped v3.0.5. * Fixed build regexp for IE11. (#369) * Bumped v3.0.6 * Add build regexp IE11 (isInteger) (#389) * Ignore airtap and travis configs (#388) * Ignore airtap and travis configs * Remove .zuul.yml from ingore list * Updated to @babel/core@7, node 10.14.2 (#393) * Updated to @babel/core@7, node 10.14.2 Fixes: nodejs/readable-stream#392 * use @babel/polyfill in the tests * Fixed missing babel deps * test passing on Node 8. * Added yarn.lock to .gitignore * Bumped v3.1.0. * Use @babel/preset-env exclusively (#395) * Fix build script to support Windows * Use `@babel/preset-env` exclusively * Rebuild node 10.14.2 * Run update-browser-errors * Bumped v3.1.1. * export * from `stream` when `process.env.READABLE_STREAM === 'disable'` (#399) * Updated to v10.15.2 (#401) * Bumped v3.2.0. * Build from Node v10.15.3 (#402) * Bumped v3.3.0. * Travis CI: Remove the deprecated sudo tag (#405) * add missing exports for browsers (#409) * add missing exports for browsers * test-stream-finished in browser * test-stream-pipeline in browser * Add node v12 to .travis.yml (#410) * Add node v12 to .travis.yml * Updated to Node v10.15.3 * Remove error checks * Bumped v3.4.0. * build/test-replacements.js: fix typo. (#415) * doc: indicate stream impl from node versions (#424) * doc: indicate stream impl from node versions * doc: fix typo Co-Authored-By: Vincent Weevers <mail@vincentweevers.nl> Co-authored-by: Vincent Weevers <mail@vincentweevers.nl> * Update to Node v10.18.1 (#420) * Update to Node v10.17.0 * Browsers might not have Symbol, from edition * Use Node 12 for airtap * test passing on browser * Update to Node 10.18.1 * fixed file patterns for build * possibly browser fix * test passing down to Node 6 * Bumped v3.5.0 * [Fix] babel's "loose mode" class transform enbrittles BufferList (#428) ```js Object.defineProperty(Object.prototype, util.inspect.custom, { set(v) { throw 'this should not happen inside readable-stream'; } }); ``` With this change, I believe the output will use [[Define]] instead of [[Set]] for https://github.com/nodejs/node/blob/c101251a95cc82142bee4637f8db6cc360a06d82/lib/internal/streams/buffer_list.js#L167, and thus no longer fail when Object.prototype is modified. * Bumped v3.6.0 * Remove Calvin from the streams wg * Add ronag to the streams wg (#430) * Remove Irina and Yosh * Add vweevers to streams wg (#431) * ci: use the shared Travis config (#444) Closes #442 * tests: add GitHub Actions CI (#418) * tests: add GitHub Actions CI * fix: add missing newline at the end of ci.yml * fix: add `pull_request` trigger to GitHub Workflows Co-Authored-By: Michaël Zasso <targos@protonmail.com> * fix: add missing `npm install` * fix: add fail-fast: false to ci.yml Co-Authored-By: XhmikosR <xhmikosr@gmail.com> * fix: Split npm install and npm run test in ci.yml Co-authored-by: Michaël Zasso <targos@protonmail.com> Co-authored-by: XhmikosR <xhmikosr@gmail.com> * Upgrade airtap and use GitHub Actions (#443) * Upgrade airtap and use GitHub Actions - Upgrade airtap to v4 - Move Sauce Labs tests from Travis to GitHub Actions - Use playwright for headless local testing * Debug * Adds Node v14 to github actions (#445) * Cleanup CI (#446) Remove debug steps from sauce workflow (#443), remove travis, and replace badges in readme. * Add usage instructions for Webpack 5 (#455) * doc: make references to primary branch be main * Update to Node 18.0.0 (#471) * feat: Updated build system and removed legacy browsers compatibility. * feat: Update to Node 17.9.0. * feat: Updated to Node 18.0.0 and simplified errors and primordials. * test: Drop SauceLabs in favor of Playwright. * feat: Restore compatibility with Node 12.x. * fix: Fixed dependencies. * feat: Do not modify globalThis. * fix: Fix path on Windows. * fix: Fixed examples. * feat: Test bundlers. * fix: Fixed node bundle on older versions. * test: Test against browserify. * test: Test against webpack. * test: Improve build scripts. * test: Test against esbuild. * test: Fixed CI. * test: Do not use node: prefix. * test: Fix CI on Windows. * test: Fix ESBuild. * Evade override mistake (#474) * v4 launch details (#472) * fixed review Signed-off-by: Matteo Collina <hello@matteocollina.com> * Update README.md Co-authored-by: Vincent Weevers <mail@vincentweevers.nl> * Update README.md Co-authored-by: Vincent Weevers <mail@vincentweevers.nl> Co-authored-by: Vincent Weevers <mail@vincentweevers.nl> * Bumped v4.0.0 Signed-off-by: Matteo Collina <hello@matteocollina.com> * ci: add GitHub token permissions for workflows (#479) Signed-off-by: Varun Sharma <varunsh@stepsecurity.io> * Lazily require abort-controller (#478) * Lazily require abort-controller, since it throws when it has been bundled, then loaded in node * Make changes to build script and re-run them. Only did 18.0.0 to minimize the changeset. * Bumped v4.1.0 Signed-off-by: Matteo Collina <hello@matteocollina.com> * Update build.mjs (#484) * Remove requirement for bundler custom config (#489) * feat: use buffer, events, process dependencies and require it * fix: added missing process file for browserify bug workaround * fix: patch code after use strict * Update to Node v18.9.0 (#490) Signed-off-by: Matteo Collina <hello@matteocollina.com> Signed-off-by: Matteo Collina <hello@matteocollina.com> * Bumped v4.2.0 Signed-off-by: Matteo Collina <hello@matteocollina.com> * chore: just build to fix whitespace issues (#498) * fix: use workaround loading process-shim for browserify (#497) * Removed Numeric separator (#491) * Bumped v4.3.0 Signed-off-by: Matteo Collina <hello@matteocollina.com> * fix: removes `Readable.fromWeb` and `Readable.toWeb` methods (#506) * removing obsolete methods from Readable class Signed-off-by: Rishabh Bhandari <rishabhbhandari6@gmail.com> * moved the logic to replacements.mjs Signed-off-by: Rishabh Bhandari <rishabhbhandari6@gmail.com> * using the replacements.mjs logic Signed-off-by: Rishabh Bhandari <rishabhbhandari6@gmail.com> --------- Signed-off-by: Rishabh Bhandari <rishabhbhandari6@gmail.com> * Update to Node v18.16.0 (#515) * Update to Node v18.16.0 Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixupo Signed-off-by: Matteo Collina <hello@matteocollina.com> * fixup Signed-off-by: Matteo Collina <hello@matteocollina.com> * format Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> * Bumped v4.4.0 Signed-off-by: Matteo Collina <hello@matteocollina.com> * chore: add node 20 to CI checks (#518) * fix: don't require `stream` package in codebase (#520) * fix: dont have require('stream') * chore: add stream importing regression tests to CI * chore: fix webpacking dist * chore: fix runner-prepare script * Bumped v4.4.1 Signed-off-by: Matteo Collina <hello@matteocollina.com> * fix: add string decoder as a dependency (#522) * Bumped v4.4.2 Signed-off-by: Matteo Collina <hello@matteocollina.com> --------- Signed-off-by: Matteo Collina <hello@matteocollina.com> Signed-off-by: Varun Sharma <varunsh@stepsecurity.io> Signed-off-by: Rishabh Bhandari <rishabhbhandari6@gmail.com> Co-authored-by: Sam Newman <sonewman@users.noreply.github.com> Co-authored-by: Ali Ijaz Sheikh <ofrobots@google.com> Co-authored-by: Calvin Metcalf <cmetcalf@appgeo.com> Co-authored-by: Calvin Metcalf <calvin.metcalf@gmail.com> Co-authored-by: Steve Mao <maochenyan@gmail.com> Co-authored-by: Shinnosuke Watanabe <snnskwtnb@gmail.com> Co-authored-by: Jesse McCarthy <git_commits@jessemccarthy.net> Co-authored-by: Matteo Collina <matteo.collina@gmail.com> Co-authored-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: RangerMauve <RangerMauve@hotmail.com> Co-authored-by: Haroen Viaene <fingebimus@me.com> Co-authored-by: Esref Durna <edurna@gmail.com> Co-authored-by: Brian White <mscdex@mscdex.net> Co-authored-by: lrlna <shestak.irina@gmail.com> Co-authored-by: Moshe Kolodny <mkolodny@google.com> Co-authored-by: Rouven Weßling <me@rouvenwessling.de> Co-authored-by: Rick Waldron <waldron.rick@gmail.com> Co-authored-by: Vincent Weevers <mail@vincentweevers.nl> Co-authored-by: Gabriel "_|Nix|_" Schulhof <gabriel.schulhof@intel.com> Co-authored-by: Douglas Wilson <doug@somethingdoug.com> Co-authored-by: Jimmy Wärting <jimmy@warting.se> Co-authored-by: Julien Mounier <mounie_a@epita.fr> Co-authored-by: Steven <steven@ceriously.com> Co-authored-by: Paul Taylor <paul.e.taylor@me.com> Co-authored-by: cclauss <cclauss@me.com> Co-authored-by: Angelo Veltens <angelo.veltens@codecentric.de> Co-authored-by: XhmikosR <xhmikosr@gmail.com> Co-authored-by: kumavis <kumavis@users.noreply.github.com> Co-authored-by: Jordan Harband <ljharb@gmail.com> Co-authored-by: Robert Nagy <ronagy@icloud.com> Co-authored-by: Dominykas Blyžė <hello@dominykas.com> Co-authored-by: Tierney Cyren <accounts@bnb.im> Co-authored-by: Michaël Zasso <targos@protonmail.com> Co-authored-by: Michael Dawson <mdawson@devrus.com> Co-authored-by: Paolo Insogna <paolo@cowtech.it> Co-authored-by: Mark S. Miller <erights@users.noreply.github.com> Co-authored-by: Varun Sharma <varunsh@stepsecurity.io> Co-authored-by: Joe Hildebrand <joe+github@cursive.net> Co-authored-by: Colin Ihrig <cjihrig@gmail.com> Co-authored-by: Thomas Bergwinkl <bergos@users.noreply.github.com> Co-authored-by: jaxoncreed <jaxoncreed@gmail.com> Co-authored-by: Rishabh Bhandari <41220684+RishabhKodes@users.noreply.github.com> Co-authored-by: Jesse Wright <63333554+jeswr@users.noreply.github.com>

mcollina requested review from mscdex and calvinmetcalf June 21, 2017 18:11

nodejs-github-bot added the stream Issues and PRs related to the stream subsystem. label Jun 21, 2017

mcollina changed the title ~~Fix finish regression~~ stream: fix finish regression Jun 21, 2017

mcollina changed the title ~~stream: fix finish regression~~ stream: finish must always follow error Jun 21, 2017

mcollina force-pushed the fix-finish-regression branch from 85bfefa to 057339d Compare June 21, 2017 18:14

mcollina mentioned this pull request Jun 21, 2017

2.3.1 broke vinyl-fs test suite nodejs/readable-stream#296

Closed

calvinmetcalf approved these changes Jun 21, 2017

View reviewed changes

mcollina mentioned this pull request Jun 21, 2017

v8.2.0 proposal #13744

Merged

gibfahn mentioned this pull request Jun 21, 2017

stream: emit finish when using writev and cork #13195

Closed

3 tasks

addaleax approved these changes Jun 21, 2017

View reviewed changes

addaleax mentioned this pull request Jun 21, 2017

v8.1.3 proposal #13861

Merged

mcollina added a commit to nodejs/readable-stream that referenced this pull request Jun 21, 2017

Forward-port of nodejs#13850

03aaa3b

nodejs/node#13850

This was referenced Jun 21, 2017

mark vinyl-fs as non-flaky nodejs/citgm#444

Open

Forward-port of nodejs#13850 nodejs/readable-stream#298

Merged

jasnell approved these changes Jun 21, 2017

View reviewed changes

tuananh mentioned this pull request Jun 22, 2017

NodeJs v8 stream.push() after EOF koajs/compress#60

Closed

mcollina added a commit to nodejs/readable-stream that referenced this pull request Jun 22, 2017

Forward-port of nodejs#13850

3e7199e

nodejs/node#13850

mcollina added a commit to nodejs/readable-stream that referenced this pull request Jun 22, 2017

Forward-port of nodejs#13850

51a9cd4

nodejs/node#13850

cjihrig approved these changes Jun 22, 2017

View reviewed changes

mcollina closed this Jun 22, 2017

mcollina deleted the fix-finish-regression branch June 22, 2017 22:39

MylesBorins added the lts-watch-v6.x label Jul 17, 2017

mcollina mentioned this pull request Jul 18, 2017

lookup: add hapi nodejs/citgm#436

Merged

1 task

mcollina added baking-for-lts PRs that need to wait before landing in a LTS release. and removed lts-watch-v6.x labels Jul 18, 2017

MylesBorins removed the baking-for-lts PRs that need to wait before landing in a LTS release. label Aug 17, 2018

renovate bot mentioned this pull request Jun 6, 2021

Update dependency readable-stream to v3 - autoclosed haxtheweb/wp-plugin-webcomponents#16

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stream: finish must always follow error #13850

stream: finish must always follow error #13850

mcollina commented Jun 21, 2017 •

edited

Loading

mcollina commented Jun 21, 2017

calvinmetcalf commented Jun 21, 2017

phated commented Jun 21, 2017

mcollina commented Jun 21, 2017

calvinmetcalf left a comment

mcollina commented Jun 21, 2017

mcollina commented Jun 21, 2017

addaleax commented Jun 21, 2017

mcollina commented Jun 21, 2017

jasnell commented Jun 22, 2017

cjihrig commented Jun 22, 2017

mcollina commented Jun 22, 2017

mcollina commented Jun 22, 2017

MylesBorins commented Jul 17, 2017

mcollina commented Jul 18, 2017

stream: finish must always follow error #13850

stream: finish must always follow error #13850

Conversation

mcollina commented Jun 21, 2017 • edited Loading

Checklist

Affected core subsystem(s)

mcollina commented Jun 21, 2017

calvinmetcalf commented Jun 21, 2017

phated commented Jun 21, 2017

mcollina commented Jun 21, 2017

calvinmetcalf left a comment

Choose a reason for hiding this comment

mcollina commented Jun 21, 2017

mcollina commented Jun 21, 2017

addaleax commented Jun 21, 2017

mcollina commented Jun 21, 2017

jasnell commented Jun 22, 2017

cjihrig commented Jun 22, 2017

mcollina commented Jun 22, 2017

mcollina commented Jun 22, 2017

MylesBorins commented Jul 17, 2017

mcollina commented Jul 18, 2017

mcollina commented Jun 21, 2017 •

edited

Loading