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

Why do Importer methods panic instead of returning errors? #19

Open
awesomeunleashed opened this issue Sep 17, 2019 · 2 comments
Open

Comments

@awesomeunleashed
Copy link
Contributor

This seemed like a strange choice to me, as Go libraries generally return errors instead of panicking. I would like to use the Importer to import PDFs submitted to my API, but as it stands I'm going to have to recover any panics caused by bad PDF formatting so that I can inform the caller of the issue. Any thoughts on this? Would you be opposed to a v2 that returns errors instead?

@phpdave11
Copy link
Owner

This is an interesting idea for v2.

My thought was that once an error occurs, the PDF import typically cannot continue, so the importer simply panics. If the importer were to return errors instead, the importer might need a flag saying if an error had occurred, in case more Importer functions are called. After that point the importer would always return an error, saying something like "Cannot continue due to previous error".

Please feel free to discuss how you think this should work.

@awesomeunleashed
Copy link
Contributor Author

My main thoughts on that reasoning:

  1. By using a panic, the importer is forcing the application using it to end on an error--likely fine for a simple command-line utility, but not for something like a REST service that needs to stay alive.
  2. It's standard for Go libraries to never panic unless it recovers those panics itself (encoding/json does this, for example).
  3. Maybe it needs some sort of flag as you say, but I think it is reasonable to expect consumers of the library to expect that once an error has been returned by a call, future calls won't behave as expected. If a consumer ignores an error and continues execution, that's on them. What would be the likely behavior in this case? More errors?

Personally, as a consumer, I'd be happy if the only change was to change the places where the Importer panics an error to return that error instead (which would be a simple, if breaking, change).

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

2 participants