search

RxSwiftCommunity / Action

Abstracts actions to be performed in RxSwift.
MIT License
875 stars 150 forks source link

Filter out errors from `CompletableAction.completions` #236

Closed sdanny closed 3 years ago

sdanny commented 3 years ago

Problem statement CompletableAction is usually used in some repeatable manner. Each action can finish with an error especially if there's a network request or a validation process inside the workFactory. So far completions emits these errors which in turn disposes existing subscriptions to this observable.

Proposed solution Catch errors in completions.

trivial

ashfurrow commented 3 years ago

Seems reasonable, but it would be a breaking API change. I'll request some reviews from others to gather feedback 👍

mosamer commented 3 years ago

Seems reasonable, but it would be a breaking API change. I'll request some reviews from others to gather feedback 👍

LGTM as well. The change is behavioural as far as I can see, API itself is intact. AFAICT, it doesn't contradict with neither how it is named completions nor the existing documentation.

@sdanny We may need however to detail documentation a bit and add entry to CHANGELOG

ashfurrow commented 3 years ago

@mosamer thank you 🙌 I agree the shape of the API itself doesn't change, but the semantics do in a way that warrants a major version bump. The current version is 4.3.0, so the next version with this change should be 5.0.0.

mosamer commented 3 years ago

@mosamer thank you 🙌 I agree the shape of the API itself doesn't change, but the semantics do in a way that warrants a major version bump. The current version is 4.3.0, so the next version with this change should be 5.0.0.

@ashfurrow IMHO this is more of a bug fix, comparing it to elements (which emits next events values instead of complete event) this stream should not dispose upon errors. That said, I agree bumping up to 5.0.0 may be a safer choice so I am fine with it too.

rxswiftcommunity[bot] commented 3 years ago

Thanks a lot for contributing @sdanny! I've invited you to join the RxSwiftCommunity GitHub organization – no pressure to accept! If you'd like more information on what this means, check out our contributor guidelines and feel free to reach out with any questions.

Generated by :no_entry_sign: dangerJS

ashfurrow commented 3 years ago

Cool! This is released as version 5.0.0. I ran into this error from CocoaPods during pod trunk push but got around it by adding --skip-import-validation. Thanks again!