alltherooms / cached-request

Node.js module to perform HTTP requests with caching support
MIT License
61 stars 23 forks source link

Handling exception when parsing JSON file #23

Closed phiferd closed 7 years ago

phiferd commented 7 years ago

These errors are causing frequent issues in my application. I think the correct behavior is to pretend as if the cache entry was not found if it is corrupted.

SyntaxError: Unexpected end of JSON input
    at Object.parse (native)
    at ReadStream.<anonymous> (/musicoin.org/node_modules/cached-request/lib/cached-request.js:189:35)
    at emitNone (events.js:91:20)
    at ReadStream.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:974:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)

Also, another thing that should be done (I think) is use an atomic write pattern, where the newly cached output is written to /some/file/name.json.tmp, and then renamed to /some/file/name.json after a successful write. If for whatever reason the write fails, there is no way a corrupted file can appear -- it's either there or it's not.

elhoyos commented 7 years ago

Your idea of atomic writes sounds interesting in order to increase the cache hit. ATM, this change does not look like a necessary feature to invest time in, please feel free to open a PR in case you really need it.