Skip to content

stream: throw error if stream has been destroyed on _final and _write#41164

Closed
JPedroAmorim wants to merge 1 commit into
nodejs:mainfrom
JPedroAmorim:issue/39030
Closed

stream: throw error if stream has been destroyed on _final and _write#41164
JPedroAmorim wants to merge 1 commit into
nodejs:mainfrom
JPedroAmorim:issue/39030

Conversation

@JPedroAmorim
Copy link
Copy Markdown

Fixes: #39030

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 14, 2021
@mscdex
Copy link
Copy Markdown
Contributor

mscdex commented Dec 14, 2021

This needs test(s).

@ronag ronag self-requested a review December 14, 2021 07:35
@mcollina mcollina added needs-citgm PRs that need a CITGM CI run. semver-major PRs that contain breaking changes and should be released in the next major version. labels Mar 15, 2022
called = true;

state.pendingcb--;
if (!err && state.destroyed) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could move this into the if-else block below to make it a bit cleaner:

if (err) {
  // ...
} else if (state.destroyed) {
  // ...
} else if (needFinish(state)) {
  // ...
}
Copy link
Copy Markdown
Member

@ronag ronag left a comment

Choose a reason for hiding this comment

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

This looks wrong to me but I don't have time to provide proper feedback atm.

@aduh95
Copy link
Copy Markdown
Contributor

aduh95 commented Sep 8, 2022

This would need to be rebased if we still want to merge it.

@aduh95 aduh95 added the stalled Issues and PRs that are stalled. label Sep 20, 2023
@github-actions
Copy link
Copy Markdown
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

@github-actions
Copy link
Copy Markdown
Contributor

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

@github-actions github-actions Bot closed this Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. needs-citgm PRs that need a CITGM CI run. semver-major PRs that contain breaking changes and should be released in the next major version. stalled Issues and PRs that are stalled.

6 participants