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

The fail-hard nature of Observable errors lend itself ill-suited for certain applications #177

Closed
dead-claudia opened this issue Jan 16, 2018 · 15 comments

Comments

@dead-claudia
Copy link

Currently, Observables close on error. This is well-documented, and most popular libraries implementing this spec have been doing this for a while. However, this limits its utility in certain areas where fault tolerance is either helpful or even required, and several reactive libraries similarly remain alive on error (just propagating the error down the stream until caught).

  • For streaming data, Observables make perfect sense. When a stream errors, it's almost always fatal, and there's nothing you can do about it.
  • For UI view streams, uncaught errors should be fatal, since you can't do anything about it.
  • For server processing loops, it's pretty obvious you could model it as an Observable<Request> => Observable<Response>, but you can't let uncaught errors close it, for obvious reasons.
  • For UI update streams, uncaught errors should be tolerated, so the application doesn't lock up and become unresponsive.

So, my thought is this:

  • Most external sources and sinks have to shut down immediately on error, because it implies something went wrong beyond your control. This could be modeled as a combined error and complete, but it needs to be done atomically.
  • Most internal sources and sinks, including sources that are also sinks (i.e. operators), should tolerate errors, because the errors themselves are largely within your control.

So given this distinction, I feel it might be better if there was a failable variant introduced for both sending and receiving. They could still be part of the observable umbrella, but it'd open up a whole host of new possibilities.


Here's a list of some libraries I know of, and how they handle errors:

@jordalgo
Copy link

@isiahmeadows No arguments here 😄 I had opened a similar issue a while back and there was some discussion around it but I think folks lost interest in my pro "stay-open" ramblings.

@staltz
Copy link

staltz commented Jan 16, 2018

The trick is to gather all errors into a separate Observable<Error> (note next will emit error objects).

const error$ = new Subject();

Observable.of(1, 2, 3, 4, 5)
  .map(n => {
    if (n === 4) {
      throw new Error('four!');
    }
    return n;
  })
  .catch((err, caught) => {
    error$.next(err);
    return caught;
  })
  .take(30)
  .subscribe(x => console.log(x)); // 1, 2, 3, 1, 2, 3, ...

error$.subscribe(x => console.log(x)); // Error, Error, Error, ...

@benjamingr
Copy link

benjamingr commented Jan 16, 2018

I don't understand this - the error handling behavior of observables is similar to that of non-reactive code. If you want to handle an error in an Observable<Request> => Observable<Response> you can simply add a .catch to that transition which gets the error and can respond with an Observable<Response> with a 500 error.

(That is, assuming there's a switchMap somewhere on Observable<Request>::SwitchMap (Request => Observable<Response>) => Observable<Response> - you can .catch on the inner Observable<Response>)

I write code that recovers from errors with Rx all the time and it never felt weird.

@staltz
Copy link

staltz commented Jan 16, 2018

Also, a typical thrown error in plain JavaScript will abort subsequent synchronous code unless caught.

Observables follow that behavior.

@zenparsing
Copy link
Member

Also, a typical thrown error in plain JavaScript will abort subsequent synchronous code unless caught.

Yes, observable sequences intentionally have the same semantics as simple iteration (i.e. for-of).

@dead-claudia
Copy link
Author

Probably the best way to contrast this is:

  1. Closing early comes from viewing it as a form of iteration, mapping over an emitter to produce a new emitter.
  2. Tolerating errors comes from viewing it as a simple source you can transform, akin to streams.

From a type-theoretic perspective, this is the difference between Haskell's monads and arrows, where monads are a generalization of iteration, and arrows are a generalization of control flow. Similarly, mapping over an emitter is obviously iteration, but transforming the emitter could be seen as invoking a pseudo-function over each emitted value.

Also, before I continue on, just thought I'd make this clear: I'm aware that these two are technically equivalent in theory, and that you could in fact model both fail-hard and fail-soft in terms of one another:

  • Fail-soft in fail-hard: @staltz's example demonstrates how you could do this.

  • Fail-hard in fail-soft: you could do this (assuming it was using an Observable-like API):

    // Manual unsubscription to avoid a race condition
    let sub
    observable.subscribe({
        start(s) { sub = s },
        error(e) { sub.unsubscribe(); observer.error(e); observer.complete() },
        complete() { observer.complete() },
    })

@staltz

The trick is to gather all errors into a separate Observable (note next will emit error objects).

I'm aware that's an option, but it seems odd from a design standpoint that you should even need to link to a separate observable for error handling, rather than simply handling it inline.


@benjamingr

If you want to handle an error in an Observable => Observable you can simply add a .catch to that transition which gets the error and can respond with an Observable with a 500 error.

(That is, assuming there's a switchMap somewhere on Observable::SwitchMap (Request => Observable) => Observable - you can .catch on the inner Observable)

This requires that you allocate an entire Observable pipeline for each request, when nearly every request results in at most one response. For concurrent requests, this would eliminate some of the advantages of short-circuiting, it'd complicate certain forms of middleware, and it's just flat out wasteful in certain spots.* So although it might work on the front end for UI error handling, this would not work for anything where high concurrency is the norm.

(This is one of the key differences between monadic and arrow-based pipelines: memory requirements.)

* Yes, I'm aware that Koa uses a similar process, but it has a few key differences that mitigate it:

  1. It promotes the use of async functions, which require a lot less memory than callback-based promise chains.
  2. Promise chains aren't nearly as allocation-heavy as observable chains, which would result in better performance from less memory churn.
    • The Promise constructor is rarely used, even in more complex scenarios, and its constructor requires three objects to be allocated, which all reference the same internal state as flyweights.
    • The Observable constructor is often required for custom operators due to the lack of a built-in equivalent to then (which could map over all channels at once), even with simpler ones like distinct, and its constructor, which requires allocation due to not being immediately invoked, creates two new, potentially exposed objects on each subscription, one holding the subscriber and the other holding the unsubscribe hook, with neither directly sharing any state.

I write code that recovers from errors with Rx all the time and it never felt weird.

I'm more commenting on an area where this generates boilerplate, and thus becomes pretty unintuitive for those who aren't as familiar with observables. I'm familiar with them, but one of the things that frequently drives me away from them is all the complexity from the vast number of operators required to sustain them. It's also why I suggested a while back creating a syntactic variant, something that would cut down on the number of operators you need and fit more in line with how observables are used today, similar to how async functions made promises a little easier to reason about. (Of course, I'd rather shoot back to the drawing board regarding observable-specific syntax than stick with what's in that proposal there, but that's beside the point.)

@benjamingr
Copy link

From a type-theoretic perspective, this is the difference between Haskell's monads and arrows, where monads are a generalization of iteration, and arrows are a generalization of control flow

I'm not sure I get this, monads never felt like a generalization of iteration when I was writing in Haskell. Arrows and Monads both felt like a form of flow control - especially variations of the continuation monad which explicitly feels like flow control - especially with syntactic assist and do notation.

Having used both I also find the answer a bit weird to be fair.

If you want to handle an error in an Observable => Observable you can simply add a .catch to that transition which gets the error and can respond with an Observable with a 500 error.
This requires that you allocate an entire Observable pipeline for each request, when nearly every request results in at most one response.

Since an observable is like a function, chaining operators is like composing functions (rather than transforming streams) and subscribing to it is just like calling a function - I'm fine with invoking that composed function once per request.

I've found that I needed this for more things than catching and recovering from errors - for example adding timeouts, logging instrumentation and various other things.

this would not work for anything where high concurrency is the norm.

Well, I'm not sure how to respond to that because it certainly does work for production systems I've worked on that needed high concurrency as the norm. I agree it's likely not the fastest way to write code - but I'm not sure there's a performance concern of a chain-per-request. If that ever becomes a problem you can always use @staltz trick and gather errors that way.

  • Yes, I'm aware that Koa uses a similar process, but it has a few key differences that mitigate it:
    Promise chains aren't nearly as allocation-heavy as observable chains, which would result in better performance from less memory churn.

This is optimization work, theoretically an observable should be a lot cheaper to create and use than a promise since it doesn't need to cache values or keep track of subscribers.

I'm familiar with them, but one of the things that frequently drives me away from them is all the complexity from the vast number of operators required to sustain them.

I acknowledge observables are a new concept for most people and they have a hard time learning it initially (like recursion, functional languages, matrix multiplication or closures) and I think a syntactic assist should be pretty nice.

The problem is that this proposal is pretty old at this point and has been stuck at stage 1 for several years now and while I think for...on with try/catch would be a lot nicer than composing the chains "manually" it's worth pursuing after this proposal advances to stage 4 rather than now.

similar to how async functions made promises a little easier to reason about.

It's worth mentioning that IMO promises would look different if we added them with fully-specified async functions.

@dead-claudia
Copy link
Author

@benjamingr

I'm not sure I get this, monads never felt like a generalization of iteration when I was writing in Haskell.

Yeah...that was a bit inaccurate. It's really somewhere between "mapping" and "iterating". (Most key uses of observables aren't even from monadic joins - that's possible, but observables are more often traversed rather than bound together, where promises are more often joined.)

Since an observable is like a function, chaining operators is like composing functions (rather than transforming streams) and subscribing to it is just like calling a function - I'm fine with invoking that composed function once per request.

I've found that I needed this for more things than catching and recovering from errors - for example adding timeouts, logging instrumentation and various other things.

Only tangentially related, but a pipeline operator would solve most of the composability issues that currently exist with observables.

This is optimization work, theoretically an observable should be a lot cheaper to create and use than a promise since it doesn't need to cache values or keep track of subscribers.

A promise can trash its list of subscribers as soon as it's resolved, and any decent implementation does. In practice, you could allocate arrays only as necessary and model its state as a pretty concise 3-variant sum type, which would result in very little memory footprint:

enum PromiseState<T> {
    Pending {
        isHandled: bool;
        fulfills: Vec<FnOnce(T) -> ()>;
        rejects: Vec<FnOnce(T) -> ()>;
    },
    Fulfilled {
        value: T;
    },
    Rejected {
        isHandled: bool;
        value: T;
    },
}

Conversely, an observable still has to keep track of exactly one subscriber object, and can't let it go until after it errors or completes. If that subscription includes next and error both and assuming those are both lambdas, it's now occupying roughly the same space as a promise with a then callback, if not more, since it also has to track closed state, the subscription observer (while the outer observable is referenced), as well as the unsubscription function/object, if applicable.

I'll concede that pre-subscription, it's minimal, though, and it can specialize for just having one observer.

The problem is that this proposal is pretty old at this point and has been stuck at stage 1 for several years now and while I think for...on with try/catch would be a lot nicer than composing the chains "manually" it's worth pursuing after this proposal advances to stage 4 rather than now.

I'm also +1 on this, but I feel the core observable API really needs other features (e.g. a pipeline operator) before we start seriously considering most other potential issues with it. (The pipeline operator would make method chaining less of a requirement to be ergonomic, and I suspect it would dramatically change how many people use observables.)

It's worth mentioning that IMO promises would look different if we added them with fully-specified async functions.

I'd have to agree with this. Asynchrony is hard, but the async/await syntax encourages procedural code a little more than it should.

@zenparsing
Copy link
Member

Pipeline operator? You mean that thing that doesn't accept multiple arguments without an extra arrow function? :P

@dead-claudia
Copy link
Author

dead-claudia commented Jan 17, 2018

@zenparsing Yes, although I don't feel it's ideal, and it's been discussed to death in that bug as well as this bug in the pipeline operator's repo. That ship appears to have sailed at this point, so I'm not really debating it any longer. 😛

@jhusain
Copy link
Collaborator

jhusain commented Jan 29, 2018

I think it’s important that anything named “Observable” match the semantics of Iterable in the event of an error (principle of least surprise). As others have pointed out (and has been acknowledged), the catch operator can be used to handle common scenarios in which closing on error is not desired. I don’t find performance arguments compelling enough to complicate the API, particularly because I don’t think they have been demonstrated convincingly. Closing this issue for now.

@jhusain jhusain closed this as completed Jan 29, 2018
@sam-s4s
Copy link

sam-s4s commented Jul 11, 2018

Hmmm I've just run into this now. In my situation there is a subject reporting HTTP results. On success it does a next with the data. On failure it does an error with the error. In no way does an error mean that future HTTP requests are unable to be completed... so it seems counter-intuitive that the subject completes on error.

I propose a boolean setting for Subjects that prevents this (often inappropriate) behaviour...

So you could set completeOnError = false (true by default), and then it will just spit errors and values out accordingly, without closing, until you actually do a complete.

@dead-claudia
Copy link
Author

@sam-s4s

Supposedly, the correct way to handle this is to replace the active observable via .catch, but this is IMHO incredibly inefficient for what's effectively just a try/catch mechanism. It's like replacing the entire server when all you care about is not failing over an I/O error or logging a 500 if something unexpected happened, which won't scale very far. (Smaller sites can tolerate it, but mid-sized ones with tens of thousands of parallel requests would struggle if errors occur.)

Honestly, I'm coming of the impression observables are probably suboptimal for servers regardless of the outcome of this bug, because it's unclear what the response should look like (I can tell you a simple "return headers + stream" isn't always most efficient or even possible for HTTP1.1, and I'm still learning enough about HTTP2 to figure out how to address the shared connection situation).

@benjamingr
Copy link

It's like replacing the entire server when all you care about is not failing over an I/O error or logging a 500 if something unexpected happened, which won't scale very far. (Smaller sites can tolerate it, but mid-sized ones with tens of thousands of parallel requests would struggle if errors occur.)

For what it's worth that's what Node.js does. This is the behavior with callbacks and has been forever - unhandled errors terminate the server.

@dead-claudia
Copy link
Author

@benjamingr Catching exceptions isn't the same as crashing. I'm referring to the catching behavior, not the crashing behavior:

  • For event callbacks, you're only allocating a function and either a property or a possible array entry beforehand to catch an error. When the error is generated, handling it is the overhead of a dynamic function call.
  • For observables, it (usually) has to allocate the event callback regardless, and if an error occurs, you're also allocating at runtime an observable, a subscriber, and potentially a subscription observer callback + closure + an observer, if the logic is non-trivial.

If you were to make errors non-fatal, all of that overhead would disappear, which was my point.


I will reiterate this, though:

Honestly, I'm coming of the impression observables are probably suboptimal for servers regardless of the outcome of this bug [...]

This was after trying for a while trying to see how a server could fit into the world of observables, and I'm beginning to believe that observables are exactly the wrong abstraction here, and that it needs to remain much more dynamic. If anything, I'm starting to believe the server side is relying too much on static material, to the point it's getting boilerplatey and unnecessarily bloated. It's much easier to do conditional routing when it's just an if/else, rather than relying on actual variables. Also, most common "middleware" I've seen are only doing something in one spot (like static resource serving, authentication, cache control, and body parsing). There are a few exceptions like logging, but only a few, and I'd like to figure out which ones are actually common. But in general, file system and caching performance would have a much greater influence on throughput than basic string slicing, string concatenation, and small object allocation (which is often pooled by the engine to avoid GC strain).

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

No branches or pull requests

7 participants