Handle caaqm loading error

[jflasher]

@MichalCz would you be able to tell me how best to handle a loading error at https://github.com/openaq/openaq-fetch/blob/develop/adapters/caaqm.js#L39? I see we have one later for the individual stations, but right now fetch is crashing when doing this initial load (and page is unavailable). I've disabled the source for now.

Thanks!

[MichalCz]

I missed this issue. I'll take a look tomorrow, but I guess, there's just a need for some simple on(error).

[MichalCz]

@jflasher, the issue here is the error not forwarded between request and JSONStream. I'm thinking of an elegant way to do this, but for now I'd suggest:

return DataStream
  .from(
    () => {
      const response = request(options);
      const parser = JSONStream.parse('map.station_list.*');
      response.on('error', ...e => parser.emit('error', ...e));;

      return response.pipe(parser);
    }
  )

The errors on the returned parser will get picked up by scramjet, so it'd get caught in the end...

[MichalCz]

@jflasher what do you think about something like this:

DataStream
    .pipeline(
        request(url),
        JSONStream('main.*')
    )
    .map(someMapper)
    .pipe(process.stdout);

The pipeline could be used instead of DataStream.from where any number of stream-ish operations could be passed and that would forward the errors along the way, as well as synchronously return an output stream like from.

[jflasher]

@MichalCz I know this isn't exactly the same, but I like the way that async does it (http://caolan.github.io/async/docs.html#parallel). You can give it a series of functions to run and if any of them fail, it'll immediately fail the whole thing and return that error. So I don't know that the operations are getting forwarded to the other functions, but they are getting passed back somehow.

For the temporary fix, I had to use the following to get the code to run

    .from(
      () => {
        const response = request(options);
        const parser = JSONStream.parse('map.station_list.*');
        response.on('error', (e) => parser.emit('error', 'Error with loading source url.'));

        return response.pipe(parser);
      }
    )

but even with this, when I put in an invalid url to test I was seeing an error message like below which seems like it's not being nicely caught? As I write this, I realize we also need to nicely handle the case where invalid JSON is passed to the parser. I tried that as well and it looks like it's crashing rather than catching it?

$ node index.js -ds caaqm                                                                                      [±:in:∂]
2018-12-03T23:06:02.479Z - info: --- Dry run for Testing, nothing is saved to the database. ---
2018-12-03T23:06:02.498Z - info: Looking up source caaqm
2018-12-03T23:06:02.816Z - info: Getting stream for "caaqm" from "caaqm"
2018-12-03T23:06:02.901Z - error: Adapter error occurred Error: Adapter error (source: caaqm)
    at stream.catch (/Users/jflasher/workspace/openaq-fetch/lib/errors.js:186:13)
    at promise.catch (/Users/jflasher/workspace/openaq-fetch/node_modules/scramjet-core/lib/util/promise-transform-stream.js:226:36)
    at <anonymous>
    at runMicrotasksCallback (internal/process/next_tick.js:121:5)
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)

[MichalCz]

Hmm... I'll try to run this tomorrow evening and check. There is an option to put a try/catch around the error and this should work, but it should do the same with the function (as the function is internally surrounded with try/catch). One thing I do know: running a single source will throw the error and result in non-zero exit code (brilliantly engineered, lousily documented - sorry). If you run this with all the other adapters it wont crash, but it will still fall the whole adapter, which ain't nice. But I'll also check tomorrow, as I said. M.

[MichalCz]

@jflasher I'm looking at a couple of changes to prove my point. I'm currently getting the error caught, but it causes the whole process to hang - it will get all the data except our current adapter, but it would fail while it should handle the situation.

I'll have to spend some time tomorrow, I'll let you know my findings and probably I'll create a pull request.

[jflasher]

Thanks!

[MichalCz]

Hi Joe,

Please see my commit here: signicode/openaq-fetch commit: 3c471a13

It's working - meaning if you change the url of the link, it'll forward the error, but I still cannot get round the fact that the stream doesn't end and hangs there. I need to spend some quiet and focused hours on this and checking how such request should actually work.

The soonest I'll find a bulk of time is weekend, so until then if you tried to add stream.end somewhere or abort on request and you'd have luck in finding out what holds the stream we'd be one step further. I have a gut feeling, but not much more, that it may be somehow connected to my failed attempts on reading part of those requests in arpalazio.

[jflasher]

@MichalCz I looked it over for a bit, but I'm afraid I am not going to be much help here.

Relatedly, while running some tests, I saw a crash due to the eea-direct adapter URL not being found. I'm sure it was just a transient error, but I guess this is probably an issue we'll see with any of the request methods and streaming? Thanks for your help on this!

[MichalCz]

Yes. It may be that. The function wrapping solution fixes the issue (there won't be a crash), but I need to investigate the actual reasons for not finishing the streams. I'm adding the pipeline method which will make the code look better, so I'll provide a fix for all the affected adapters with it.

--

Signicode

[MichalCz]

@jflasher a new version of scramjet with a new pipeline method on all streams. It seems solve the issue that occured here and creates a nice pipeline and cleaner code.

I'm playing around the caaqm adapter a bit and will check all 6 adapters I rewritten for similar improvement and create a push request. It should be ready by tomorrow evening CET.

[jflasher]

Awesome, thanks!

[MichalCz]

Good morning @jflasher

Please see if this is a sufficient solution. :)

[jflasher]

Should be closed with #573