sharedstreets / mobility-metrics

Tools for collecting, processing, and interpreting mobility data using SharedStreets
MIT License
50 stars 17 forks source link

Add error checking for cache #61

Closed jwoyame closed 5 years ago

jwoyame commented 5 years ago

This PR does a few things that were required for error handling.

Currently, if an error occurs during the provider query, the cache.js queue will complete with an error (ignored) and the Promise will resolve as if the caching succeeded. However, the summarization process will fail later with an ENOENT: no such file or directory error for the provider's cache JSON files, since they will not exist. This makes it hard to debug provider-related issues.

This PR ensures that any error gets tagged with the provider name and is passed up to the entry point (backfill.js) where it will be printed to the console. Example: cannot query lime changes: x is not defined

In order to allow the errors to propagate up, I removed the explicit Promise from two async functions. These weren't really needed because an async function will generate an implicit Promise. Leaving them off makes the code a little cleaner, allows an error to propagate, and also fixes a potential bug in summarize.js where the resolve function was being called in a loop (the await will continue as soon as the first provider is complete).

As a side bonus, the provider queries can now report an error by calling .destroy(new Error(...)) on the stream object.

Let me know if I should adjust this in any way, thanks!

morganherlocker commented 5 years ago

Thank you, looks great!