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

End message is not sent and no errors are logged. #7

Open
piemadd opened this issue Dec 1, 2023 · 6 comments
Open

End message is not sent and no errors are logged. #7

piemadd opened this issue Dec 1, 2023 · 6 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@piemadd
Copy link

piemadd commented Dec 1, 2023

Being very similar to the example, I set up this script:

const gtfs = require('gtfs-stream')
const fetch = require('node-fetch');

fetch('https://www.transitchicago.com/downloads/sch_data/google_transit.zip')
  .then(res => {
    res.body
      .pipe(gtfs())
      .on('data', (entity) => {
        //console.log(entity)
      })
      .on('end', () => {
        console.log('end')
      })
  })

Which works as normal, logging out the entities, but what is never logged is the 'end'. Is an end message meant to not be sent?

@yocontra
Copy link
Member

yocontra commented Dec 1, 2023

Can you try using finished from the stream built-in instead of the end event? Also which node version?

@piemadd
Copy link
Author

piemadd commented Dec 1, 2023

In theory, I can. Would there be a difference?

Also node version 16.20.2

@yocontra
Copy link
Member

yocontra commented Dec 2, 2023

Yeah the underlying stream behavior has changed between 14 -> 16 -> 18, I haven't seen this issue on 16 before but the test matrix doesn't include anything newer than 15. If you want to submit a PR to move the tests to use github actions (travis ci is busted now) + include newer versions of node in the test matrix would be much appreciated, otherwise I'll put this on my TODO.

@yocontra yocontra added the bug Something isn't working label Dec 2, 2023
@piemadd
Copy link
Author

piemadd commented Dec 3, 2023

I might have the free time to tackle this within the next few days. Will let you know if I am unable to though.

@linusnorton
Copy link
Contributor

linusnorton commented Dec 3, 2023

I'm not 100% on the steps to test (I went with build, lint, test) but hopefully this is close to what you need: #8

@piemadd
Copy link
Author

piemadd commented Dec 3, 2023

Smells about right. Looking around, it seems like whatever issue I was running into was caused by some upstream package updating. Locking all of the dep versions, it works perfectly on node 20. I'll look into slowly updating stuff and will report/fix whatever the breaking change is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants