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

ngclient: decide what to do with requests (Fetcher impl) errors #1714

Closed
jku opened this issue Dec 8, 2021 · 2 comments · Fixed by #1810
Closed

ngclient: decide what to do with requests (Fetcher impl) errors #1714

jku opened this issue Dec 8, 2021 · 2 comments · Fixed by #1810
Assignees
Labels
1.0.0 blockers To be addressed before 1.0.0 release ngclient
Milestone

Comments

@jku
Copy link
Member

jku commented Dec 8, 2021

I wonder if we should handle all exceptions thrown by Fetcher implementations (and throw a generic error that clients can actually handle)?

This issue can be tested with examples/client_example/client_example.py: if e.g. server is not running there's a requests ConnectionError. All requests errors do derive from OSError so it gets handled by the example code but another fetcher implementation might not do the same.

Alternatively we can just say that Fetcher implementations should document what gets thrown (and requestsFetcher could just link to RequestException)

@jku jku added the ngclient label Dec 8, 2021
@jku
Copy link
Member Author

jku commented Jan 12, 2022

I think Fetcher should handle all exceptions, and raise a FetcherError which should derive from DownloadError -- this means client can just handle DownloadError which is what they probably want

@jku jku added the 1.0.0 blockers To be addressed before 1.0.0 release label Jan 12, 2022
@jku jku self-assigned this Jan 26, 2022
@jku jku added this to the sprint 16 milestone Jan 26, 2022
@jku
Copy link
Member Author

jku commented Jan 31, 2022

Actually, there is no need for separate DownloadError and FetcherError: they are one and same (all DownloadErrors come from fetcher). We just need to document the fetcherinterface so that the methods are only allowed to raise DownloadErrors and derivatives -- or handle that in the interface itself. RequestsFetcher needs a small modification so fetch() actually enforces this requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0.0 blockers To be addressed before 1.0.0 release ngclient
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant