Conversation
| sub:Fire(current) | ||
|
|
||
| return current | ||
| end, function(...) |
There was a problem hiding this comment.
we can pass sub:GetFailComplete() here. Note I think it's fine to complete this here, but a failure could cause other observables in the blend tree to also fail. Most of Nevermore doesn't really leverage the failure state that much.
There was a problem hiding this comment.
If you think it's safe to merge this, I'm down for it.
There was a problem hiding this comment.
I wanted the completion, I didn't realise this would could pass failures. I was only vaguely aware subscriptions had this - I know for certain I'm not handling it!
Though it's a breaking change, I would want to propagate this. I think hiding errors is bad. But silently failing is also bad, because I'm not handling this case and wouldn't know why my subscription stopped. I think subscriptions should print a warning (similar to promises) for an unhandled failure state (i.e. no failCallback), and then this should be merged?
|
Side note: Rx.EMPTY never completes I believe. |
|
If propagating failure is contentious, is it possible to merge just the first of these commits for now? |
nilfrom the observable. We know that it will never evaluate to anything interesting, so there's no point creating a brio (which are not free). (Should also probably skip creating a brio if the passed observable is complete and evaluates to nil, i.e. ==Rx.EMPTY?).