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

Support for duplex streams? #7

Closed
stevenvachon opened this issue Feb 22, 2021 · 9 comments
Closed

Support for duplex streams? #7

stevenvachon opened this issue Feb 22, 2021 · 9 comments

Comments

@stevenvachon
Copy link

I've had success in the past passing in a Transform stream, but a duplex stream is not resolving the promise.

@Janpot
Copy link
Contributor

Janpot commented Feb 23, 2021

Will take a look. If you have a small snippet that reproduces the issue, that would be helpful

@Janpot
Copy link
Contributor

Janpot commented Feb 23, 2021

@stevenvachon in 2.0.0 I rewrote the logic with the native stream primitives, this might solve it.

@stevenvachon
Copy link
Author

Hmm, I get nothing with 2.0.0:

import got from 'got';
import parseRobots from 'robots-txt-parse';

const getRobotsTxt = url => new Promise((resolve, reject) => {
  got.stream(url, {
    method: 'get',
    throwHttpErrors: false
  })
    .on('error', reject)
    .on('response', async stream => resolve(await parseRobots(stream)));
});

getRobotsTxt('https://svachon.com/robots.txt')
  .then(console.log)
  .catch(console.error);

@Janpot
Copy link
Contributor

Janpot commented Feb 23, 2021

This seems to work for me:

import got from 'got';
import parseRobots from 'robots-txt-parse';

const getRobotsTxt = async url => parseRobots(got.stream(url));

getRobotsTxt('https://svachon.com/robots.txt').then(console.log, console.error);

parseRobots should propagate the underlying stream errors to the returned Promise, so no need to wrap it with a promise and listen for the error event.

As for the original code, the following doesn't print anything for me, so I think it's something in got, and not in robots-txt-parse

import got from 'got';

const printRobotsTxt = url => {
  got.stream(url)
    .on('error', console.error)
    .on('response', async stream => {
      console.log('response received')
      stream.on('data', console.log)
    });
};

printRobotsTxt('https://svachon.com/robots.txt')

@stevenvachon
Copy link
Author

I'm using on('redirect'), so I can't return too early. I'll keep digging, but v2 of this package works with got v10 but not v11.

@stevenvachon
Copy link
Author

Thanks

@szmarczak
Copy link

so I think it's something in got, and not in robots-txt-parse

@Janpot sindresorhus/got#223 (comment)

@Janpot
Copy link
Contributor

Janpot commented Mar 5, 2021

@szmarczak Is there a reason why stream.on('data', console.log) in

    .on('response', async stream => {
      stream.on('data', console.log)
    });

doesn't put stream in flowing mode? It's quite counterintuitive.

@szmarczak
Copy link

@Janpot you're using the IncomingMessage stream here. It is passed to Got stream so you should be reading from the Got stream instead. Otherwise it waits so you consume the Got stream first. It's the default Node.js piped stream behaviour.

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

3 participants