Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Don't match cancellation in NonFatal #308

Merged
merged 4 commits into from Jan 22, 2021
Merged

Don't match cancellation in NonFatal #308

merged 4 commits into from Jan 22, 2021

Conversation

nomisRev
Copy link
Member

@nomisRev nomisRev commented Jan 20, 2021

In Kotlin 1.4 CancellationException was added as a cancellation system for coroutines.
We thus shouldn't match CancellationException in NonFatal, or it would be prone to being swallowed in user code.

I.e. if fetchUser is cancelled it throws CancellationException, and in the below snippet it could be easily swalled and mapped to a default error type. This would easily cause bugs, and we should let CancellationException propagate through the program so it can cancel any outer scopes.

val user = Either.catch {
  fetchUser()
}.mapLeft { DomainError.UnkownError(it) }

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

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

👌

@raulraja raulraja requested a review from a team January 21, 2021 11:44
@nomisRev nomisRev merged commit 94f8dd9 into master Jan 22, 2021
@rachelcarmena rachelcarmena deleted the sv-non-fatal branch February 10, 2021 17:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants