-
-
Notifications
You must be signed in to change notification settings - Fork 151
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
[rstream] Revised error handling logic #281
Comments
postspectacular
added a commit
that referenced
this issue
Mar 9, 2021
- add `ErrorHandler` type, update to return boolean - update `ISubscribable`, `ITransformable` to only refer to `ISubscription` interface (rather than `Subscription` class itself) - refactor `Subscription.next()`, add `.dispatchXform()` - update various error handlers (add return values) - update tests
postspectacular
added a commit
that referenced
this issue
Mar 10, 2021
- replace old `Subscription` class w/ what was recently `Sub2` (removed) - update/fix done(), subscribe()/unsubscribe() logic - update related constructs (Stream, StreamSync, MetaStream, etc.) - update Stream ctor (and factory fns) to support error handler opts arg - update Timeout error dispatch - fix typehints
Closing for now, since already implemented & published. Feel free to reopen or create new issue(s)... |
This was referenced Mar 17, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
About
The existing error handling logic in thi.ng/rstream has been somewhat peculiar (to say the least) and has been source of various confusions and unexpected outcomes. The #279 PR is aiming to change that (among other things), but I thought it best to use this separate issue here to document the existing problems, as well as the new logic getting implemented:
Subscriptions are state machines
The following diagram illustrates the various states a
Subscription
(as base type for all other constructs in rstream) can be in:A subscription starts in an
IDLE
state, goes intoACTIVE
once the first child sub is attached (and vice versa in the child) and then depending on the nature of the upstream source, the presence of errors or forced unsubscribing, eventually ends in one of theERROR
orUNSUBSCRIBED
states. TheDONE
state will only be temporarily active (during the execution of the.done()
handler) iff the upstream source is finite and completes naturally.Child subscriptions can only be added whilst the parent sub is in
IDLE
orACTIVE
states. Likewise for child subs themselves. In all other cases.subscribe()
will throw an illegal state error.The error handling discussed for the rest of this issue is about how to deal with errors in subscriptions handlers & value processing/transformations. The state machine is just useful to better understand the implications of a sub going into the
ERROR
state...Issues w/ existing error handling
The code below creates a simple linear topology (pipeline) of 4 nodes:
Example 1
Recursive error propagation
Once we start pushing a new value into
a
, thea2.next()
handler will receive that value and throw an error. Sincea2
itself has no error handler, the error will be caught by the wrapping subscription's own error handler, which then attempts to recursively call error handlers on all of its child subscriptions (which of course is objectively wrong, but the approach did make some sense, depending on use case):It's obvious, that this is far from correct behavior and there are various resulting secondary problems arising from that. In short, this is a limitation/result of the way child subscriptions have been implemented so far... The
a3
&a4
subs have nothing directly to do with the error ina2
, yet their (optional) error handlers are being invoked recursively. Even worse, there's no logic to ensure an error is only handled once (e.g. by the first downstream handler found) and there's also currently no provision to detect if an error handler could successfully respond to (or recover from) a given error... This causes all downstream nodes (froma2
onwards) to go into theERROR
state and is then stopping them from receiving any future values...Transducer error handling
Errors can not just be caused during execution of a sub's
next()
handler, but also already whilst executing a sub's optional transducer, e.g. here replacing the abovea2
with and yielding the same error behavior as above.Teardown with/without error
Depending on each subscription's configuration (via
closeIn
,closeOut
options), triggering an error state can (and with default options it should) cause a recursive partial teardown of the upstream data flow branch the sub in question belongs to. At the very least, a subscription detaches itself (via.unsubscribe()
) upon entering the error state.The
closeOut
mode of a parent sub dictates the parent's behavior when a child sub unsubscribes. By the default, the parent itself unsubscribes from its parent once the last child has disconnected (i.e.CloseMode.LAST
). However, in the current implementation this behavior is not fully respected.With logging enabled, this teardown logic becomes more visible:
Example 2
For comparison, replacing the above line
a.next(1)
witha4.unsubscribe()
yields similar (minus the error handling output). In both cases, onlya4
's parent (a3
) is impacted, but doesn't propagate further (wrong!). Note that this doesn't impact the logical behavior of the entirea
...a4
stream construct (a2
,a3
,a4
wont be receiving any future values), but the point of teardown is resource/memory usage and this isn't handled properly (but already fixed by #279).Revised error handling logic
The changes introduced in #279 are addressing all of the above issues:
.transform()
child subs).unsubscribe()
(due to errors or without)Using the same code from Example 2 above, the new behavior produces the following output:
As shown, due to
a2
's lack of user provided error handler, the wrapping subscription will catch the error, puta2
intoERROR
state and then unsubscribea2
from its parent (a
). Becausea2
wasa
's only child,a
itself will shutdown (due to defaultCloseMode.LAST
). The result states for all 4 subs are:Error handlers
The most important aspect of the new logic is that only ever a single error handler will be used now and it's always only either the user provided handler or, as fallback, that of the wrapping
Subscription
type. The above result states also show that onlya2
is put into theERROR
state, essentially constituting a cut of the data flow.Furthermore, error handlers MUST return a boolean result to indicate if the error could be successfully handled/recovered from (
true
) or failed to do so (false
). In the latter case, the wrapping sub's own error handler will be used and then trigger the sub going into theERROR
state. If the (user) handler returnstrue
, the sub remainsACTIVE
.Transducer error handling
For transducer-only subscriptions created via
.transform()
or.map()
an optional error handler can be provided via the final options object arg:The
.map()
and.transform()
methods are merely syntax sugar for this type of.subscribe()
call:True recursive teardown
Regarding recursive upstream teardown, again replacing
a.next(1)
witha4.unsubscribe()
will teardown the entire dataflow pipeline (in this case):Result states:
The teardown also works in opposite direction, e.g. here triggering
a.done()
in place ofa4.unsubscribe()
. First.done()
is invoked in a depth-first manner, which then causes the same recursive upstream teardown once the last node in the pipepline (a4
) has been reached...Result states:
Structural internal differences
There're several key interfaces used in thi.ng/rstream related to subscriptions:
ISubscriber
- describes the shape of a raw user subscription object of{next, error, done}
handlers. Such an object can be given to.subscribe()
, but will always be wrapped in a realISubscription
instanceISubscription
- the name of the interface describing key aspects of aSubscription
, i.e. a combined interface ofIDeref
,ISubscriber
,ISubscribable
,ITransformable
. This interface is fully implemented by theSubscription
class, which also acts as base class for other subscription types (i.e.Stream
,StreamSync
,PubSub
etc.)ISubscribable
- merely describes the contract of the various signatures of the.subscribe()
methodITransformable
- describes the contract of the various sigs of the.transform()
methodThe current implementation of
Subscription
doesn't differentiate between an optionally givenISubscriber
(which is wrapped by this subscription instance) and other child subscriptions attached via.subscribe()
. It's primarily this lack of distinction which is responsible for the semi-erroneous behavior(s) described in the beginning.In the rewritten
Subscription
(a9e4040), a wrappedISubscriber
is handled (and stored) separately from other child subs, allowing for all the improvements listed above.This new distinction also enables user error handling of stream source constructs and the
stream()
factory function now supports an optional error handler:Summary
The diagrams hopefully illustrate the differences between the old and new approaches. In both cases, we use code from Example 1 (with an additional sibling of
a3
) and show the states after an uncaught error occurred ina2
:The lack of connections between
a3
->a4
(in the old) anda
->a2
(in the new version) indicate that the connection between nodes/subs has been severed....transform()
subs)The text was updated successfully, but these errors were encountered: