staeco / gtfs-stream

Streaming GTFS and GTFS-RT parser for node
MIT License
30 stars 6 forks source link

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

Open piemadd opened 9 months ago

piemadd commented 9 months ago

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 commented 9 months ago

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

piemadd commented 9 months ago

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

Also node version 16.20.2

yocontra commented 9 months ago

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.

piemadd commented 9 months ago

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 commented 9 months ago

I'm not 100% on the steps to test (I went with build, lint, test) but hopefully this is close to what you need: https://github.com/staeco/gtfs-stream/pull/8

piemadd commented 9 months ago

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.