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

Unify error handling #29

Merged
merged 9 commits into from
Nov 2, 2023
Merged

Unify error handling #29

merged 9 commits into from
Nov 2, 2023

Conversation

rucek
Copy link
Contributor

@rucek rucek commented Oct 27, 2023

  • mapParScope doesn't rethrow anymore
  • pipeTo doesn't rethrow when receiving from upstream fails
  • updated docs on error propagation, with an example based on mapParUnordered

@rucek rucek requested a review from adamw October 27, 2023 20:28
@rucek rucek marked this pull request as ready for review October 30, 2023 08:53
README.md Outdated
@@ -628,10 +628,76 @@ exception being thrown there.
Won't this design cause upstream channels / sources to operate despite the consumer being gone (because of the
exception)?

No: the exception should cause the containing scope to finish (or a supervised fork to fail), cancelling any forks
that are operating in the background. Any unused channels can then be garbage-collected.
It depends on two factors:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would mention beforehand that there are two possible designs, of which we picked one, as otherwise this might make the impression that error handling in Ox is dependent on the factors below (which it isn't)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact ... such a discussion would be better placed in and ADR. Maybe this should be moved there? ADRs can be as simple as a text file, e.g. here: https://github.com/softwaremill/tapir/tree/master/doc/adr

The docs can then simply state the current state of affairs, keeping the design discussions separate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, moved the discussion to an ADR.

@rucek rucek requested a review from adamw October 30, 2023 11:47
@rucek rucek merged commit bbb64ea into master Nov 2, 2023
4 checks passed
@rucek rucek deleted the unify-error-handling branch November 2, 2023 08:44
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.

3 participants