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

pipeline and duplex having multiple error as well as not calling the pipeline function on some cases #45522

Open
rluvaton opened this issue Nov 19, 2022 · 7 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@rluvaton
Copy link
Member

rluvaton commented Nov 19, 2022

Version

v18.12.1 and 16.18.1 and 16.10

Platform

Darwin Razs-MBP 22.1.0 Darwin Kernel Version 22.1.0: Sun Oct 9 20:15:09 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T6000 arm64

Subsystem

stream

What steps will reproduce the bug?

Tested on:

  • 18.12.1
  • 16.18.1
  • 16.10.0

Functions that will be used in the examples

function getData() {
  return Readable.from(['a', 'b', 'c', 'd']);
}

async function* stopAfterFirst(stream) {
  for await (const chunk of stream) {
    yield chunk;
    return;
  }
}

async function* justListen(stream) {
  for await (const _ of stream) ;
}

function createLogPipelineResult(prefix) {
  prefix = prefix ? `${prefix}: ` : '';
  return function logPipelineResult(e) {
    if(e) {
      console.log(`${prefix}pipeline error`, e);
    } else {
      console.log(`${prefix}pipeline succeeded`)
    }
  }
}

The following are the problematic cases:

function duplexAndNotLast() {
  pipeline(
    getData(),
    Duplex.from(stopAfterFirst), // <- Wrapped with duplex
    justListen,
    createLogPipelineResult('duplexAndNotLast'),
  ).on('error', createLogPipelineResult('duplexAndNotLast on error'));

  // Node 18.12.1:
  // => duplexAndNotLast: pipeline error AbortError: The operation was aborted

  // Node 16.18.1 and 16.10.0:
  // => no log at all
}

function noDuplexAndNotLast() {
  pipeline(
    getData(),
    stopAfterFirst, // <- Not wrapped with duplex
    justListen,
    createLogPipelineResult('noDuplexAndNotLast'),
  ).on('error', createLogPipelineResult('noDuplexAndNotLast on error'));

  // Node 18.12.1:
  // => noDuplexAndNotLast: pipeline succeeded

  // Node 16.18.1 and 16.10.0:
  // => noDuplexAndNotLast: pipeline error Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
}

function duplexAndLast() {
  pipeline(
    getData(),
    Duplex.from(stopAfterFirst), // <- Wrapped with duplex and last
    createLogPipelineResult('duplexAndLast'),
  ).on('error', createLogPipelineResult('duplexAndLast on error'));


  // Node 18.12.1:
  // not logging **at all**

  // Node 16.18.1 and 16.10.0:
  // not logging **at all**
}

function noDuplexAndLast() {
  pipeline(
    getData(),
    stopAfterFirst, // <- Not wrapped with duplex and last
    createLogPipelineResult('noDuplexAndLast'),
  ).on('error', createLogPipelineResult('noDuplexAndLast on error'));

  // Node 18.12.1:
  // => noDuplexAndLast: pipeline succeeded

  // Node 16.18.1 and 16.10.0:
  // => noDuplexAndLast: pipeline error Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
}

How often does it reproduce? Is there a required condition?

every time

What is the expected behavior?

No response

What do you see instead?

added in each function in What steps will reproduce the bug?

Additional information

Maybe it's related to #44026

@rluvaton rluvaton changed the title pipeline and duplex having multiple error as well as not calling the pipeline function pipeline and duplex having multiple error as well as not calling the pipeline function on some cases Nov 19, 2022
@rluvaton
Copy link
Member Author

rluvaton commented Nov 19, 2022

cc @nodejs/streams

(It doesn't work 😢 )

@rluvaton
Copy link
Member Author

Trying to fix it :)

@anonrig
Copy link
Member

anonrig commented Nov 19, 2022

CC @nodejs/streams

@anonrig anonrig added the stream Issues and PRs related to the stream subsystem. label Nov 19, 2022
@rluvaton
Copy link
Member Author

So what I've found so far (testing on node v20.0.0-pre):

the duplexAndNotLast was aborted cause of ERR_STREAM_PREMATURE_CLOSE, do we want to propagate this error?

@mcollina
Copy link
Member

I don't understand what is the specific bug you want to fix.

@rluvaton
Copy link
Member Author

I'm confused 😖 basically there are inconsistencies between node versions relating to pipeline
and bug with pipeline and duplex, currently I'm trying to fix #45534

@mcollina
Copy link
Member

There have been multiple changes between v16 and v18, if you want to know what exactly, you'd need to dig into the v17 and v18 semver-major release notes. Note that there might also be fixes that have not been backported to v16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants