Skip to content

Feat: Exception handling #267

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Fadarrizz
Copy link

@Fadarrizz Fadarrizz commented Mar 23, 2025

This PR adds exception handling to the downloader and allows middleware to act upon caught exceptions.

Personally, I need this for handling Javascript (Browsershot) exceptions, so I can retry them. In order to act upon thrown exceptions during the request being sent, exception handling is needed at the downloader level.

@Fadarrizz Fadarrizz requested a review from ksassnowski as a code owner March 23, 2025 20:24
@Fadarrizz Fadarrizz marked this pull request as draft April 15, 2025 20:17
@Fadarrizz Fadarrizz force-pushed the exception-handling branch from 2e6187b to 4bd1829 Compare April 16, 2025 14:38
@Fadarrizz Fadarrizz force-pushed the exception-handling branch from 4bd1829 to 3b0e537 Compare April 16, 2025 14:41
@Fadarrizz Fadarrizz marked this pull request as ready for review April 16, 2025 14:45
@Fadarrizz Fadarrizz marked this pull request as draft April 16, 2025 18:44
@Fadarrizz
Copy link
Author

There is one problem I'm still solving: throwing the exception when it hasn't been handled.

@Fadarrizz Fadarrizz marked this pull request as ready for review April 26, 2025 10:43
@Fadarrizz
Copy link
Author

@ksassnowski I implemented a feature that throws exceptions when they are not handled by any middleware.

To achieve this, we need to track whether an exception has been handled, so I decided to wrap exceptions with a RequestException and maintain the handled state within that class.

I would appreciate it if you could review this and share your feedback!

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

Successfully merging this pull request may close these issues.

1 participant