Better error Handling

[Monirzadeh]

This Pull request try to remove panic from the code.
it should finally should
close #49
close #63

TODO:

• Remove all panic [31 / 31]
• Update unit test
• fix macos error
  *this todo list will update in future

@bmaupin it is not complete yet. I want remove all panic form code step by step. please review current state so i can understand your thought about this.
i will remove more unneeded panic from code in future.

[bmaupin]

Oh wow, thanks for starting this! This is much needed. I like your approach of doing it a little bit at a time.

I took a quick look, and yes, my general thought was basically to replace all of the usages of panic with error codes.

For now I only have one comment, regarding https://github.com/pkg/errors ; this package is no longer maintained and the GitHub repo has been archived.

From what I can tell, it looks like it wraps errors to provide context, which sounds great. But since that package isn't maintained, I'm hesitant to add it to this project. You can see at this issue that lots of other projects are abandoning it: pkg/errors#245

I think it would be best to either find an alternative or just leave it out. In theory if the error messages are descriptive enough, maybe we don't need it? This library is simple enough that maybe it could manage without. For instance, it looks like you're essentially wrapping errors yourself (without this library) when you do things like this:

return nil, fmt.Errorf("Can't create NewEpub: %s", err)

I'll try to take a closer look at this PR when I get a chance. Right now everything's blocked by the broken tests so I'd like to get #66 merged before merging anything else.

Thanks!

[Monirzadeh]

OK.
thanks for feedback so i continue this approach and working on a better alternative for error package.
if it is not a problem i will ping you here to review this in small chunk. or if you set yourself as reviewer on this PR i can Request review with that.

[fmartingr]

For now I only have one comment, regarding https://github.com/pkg/errors ; this package is no longer maintained and the GitHub repo has been archived.

We have this due in shiori ourselves. In order to wrap errors (the "new" way) you can use the %w denominator when creating an error: fmt.Errorf("error 1: %w", err)

[Monirzadeh]

@bmaupin remain 4 panic in write.go can you explain mere about them?

[bmaupin]

@bmaupin remain 4 panic in write.go can you explain mere about them?

It looks like the first one will get called if there's an error when the file being added to the zip file gets closed.

It looks like the others will get called if there's an error when the zip file writer is being closed due to a previous error in the process of writing the zip file.

[coveralls]

coverage: 87.105% (-0.9%) from 88.0% when pulling ca5c88f on Monirzadeh:better-error-handling into 45f45a4 on bmaupin:main.

[Monirzadeh]

@bmaupin because you want small change do you have any problem with new change?

[Monirzadeh]

@bmaupin can we have an new release after this PR merge?

[fmartingr]

@bmaupin can we have an new release after this PR merge?

Remember that you can start working using the commit sha as version in the dependencies until this PR is merged and released. Once the release is made we can point it towards the proper release.

[Monirzadeh]

yes i do same things in go-shiori/shiori@49e8ba8 some bug detected that should be solve too :)