exif-parser icon indicating copy to clipboard operation
exif-parser copied to clipboard

Callbacks and proper error handling

Open llaakso opened this issue 8 years ago • 7 comments

This module seems handy but it crashes in node.js environment:

node_modules/exif-parser/lib/jpeg.js:12 throw new Error('Invalid JPEG section offset'); ^

Error: Invalid JPEG section offset at Object.module.exports.parseSections (/home/xu/node_modules/exif-parser/lib/jpeg.js:12:11)

In node you would avoid this by using callbacks to indicate error.

llaakso avatar Mar 21 '16 10:03 llaakso

I think that shouldn't be an error after all, just a 'warning'. The module menages to get some data from broken images as well, but with this throw, those data are lost.

bpatrik avatar Jul 16 '17 14:07 bpatrik

Thank you for your suggestion.

As this is not an async API, adopting the node pattern of (err, result) makes little sense to me. The callback is used for the low level API as a visitor pattern, to avoid creating temporary arrays.

Also, it's way easier to just throw on unrecoverable errors (like an invalid JPEG section offset) than to have error handling at every layer of the callstack.

It's also easy enough to just slap a try around it, just like you would with JSON.parse.

You do have a point that it should be documented that the library will throw on invalid data, I just added this to the README.

bwindels avatar Jul 26 '18 21:07 bwindels

Hmm, as @bpatrik mentioned, it probably shouldn't throw once it has already found the exif section, and just stop parsing once all information has been found.

bwindels avatar Jul 26 '18 21:07 bwindels

I agree that this module should not throw an error at all. I've used hundreds of modules but don't know any that requires a try-block somewhere.

MickL avatar Apr 11 '19 08:04 MickL

Hi @bwindels do you have a plan to address this? I like your proposal in https://github.com/bwindels/exif-parser/issues/13#issuecomment-408247330 .

kwaping avatar Sep 06 '19 16:09 kwaping

Don't have time to work on this in the near future, sorry. Happy to review PRs though.

bwindels avatar Sep 09 '19 09:09 bwindels

I know this is 4 years old post but I just tried this library and encountered this error trying to do batch read with array that also had directory names inside.

Mulperi avatar May 30 '20 18:05 Mulperi