phpdave11 / gofpdi

Go Free PDF Document Importer
MIT License
119 stars 59 forks source link

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

Open awesomeunleashed opened 5 years ago

awesomeunleashed commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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).