Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callbacks and proper error handling #13

Open
llaakso opened this issue Mar 21, 2016 · 7 comments
Open

Callbacks and proper error handling #13

llaakso opened this issue Mar 21, 2016 · 7 comments

Comments

@llaakso
Copy link

llaakso commented Mar 21, 2016

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.

@bpatrik
Copy link

bpatrik commented Jul 16, 2017

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.

@bwindels
Copy link
Owner

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
Copy link
Owner

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 bwindels reopened this Jul 26, 2018
@MickL
Copy link

MickL commented Apr 11, 2019

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.

@kwaping
Copy link

kwaping commented Sep 6, 2019

Hi @bwindels do you have a plan to address this? I like your proposal in #13 (comment) .

@bwindels
Copy link
Owner

bwindels commented Sep 9, 2019

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

@Mulperi
Copy link

Mulperi commented May 30, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants