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

Can't return a Duplex stream from a remote node #325

Closed
felipegcampos opened this issue Jul 12, 2018 · 3 comments
Closed

Can't return a Duplex stream from a remote node #325

felipegcampos opened this issue Jul 12, 2018 · 3 comments

Comments

@felipegcampos
Copy link
Contributor

felipegcampos commented Jul 12, 2018

Expected Behavior

It should return a Duplex stream from a remote node.

Failure Information

You can find details on how to duplicate the issue in the repo https://github.com/felipegcampos/azure-stream-moleculer-issue

Context

  • Moleculer version: 0.13
  • NodeJS version: 9
  • Operating System: Linux Mint 64-bit

Failure Logs

>> ERROR: stream.pipe is not a function (NodeID: felipe-xps-15-9550-24145)
TypeError: stream.pipe is not a function
    at Service.get (/home/felipe/Documents/Workspace/moleculer/azure-stream-moleculer-issue/assets.service.js:11:14)
    at <anonymous>

Solution

Issue is here:
if (data && typeof data.on === "function" && typeof data.read === "function" && typeof data.pipe === "function") {

Unless you have reasons to do not use it, we should just check with instanceof.

if (data && data instanceof Stream)

or if you want to make sure it's a Readable stream we could also use data.readable === true. Duplex streams not always seems to have the read function but it still working as Readable stream.

or even:

if (data && typeof data.on === "function" && typeof data.pipe === "function" && data.readable === true)

This implementation for example inherits from Stream and implements its own Duplex. Although it's Readable and Writable, it would fail for Readable, Writable, Duplex and Transform instanceof checks.

Share your thoughts,
Tks

@icebob
Copy link
Member

icebob commented Jul 13, 2018

I used this solution instead of instanceof Stream due to https://stackoverflow.com/a/23885304/129346
But data.readable === true seems better. Thanks the PR!

@felipegcampos
Copy link
Contributor Author

Yes. I've checked this thread as well. There are some ways to try to determine if it's actually a Readable stream. Take a look at this project here. They use stream.readable !== false && typeof stream._read === 'function' && typeof stream._readableState === 'object';. But it also return false when checking the azure ChunkStream as readable. I'm not saying the azure ChunkStream is correct but it works as a Readable stream as well.

@icebob
Copy link
Member

icebob commented Jul 13, 2018

Released in 0.13.1

@icebob icebob closed this as completed Jul 13, 2018
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

No branches or pull requests

2 participants