Exception handling in .map, .flatMap and other operations

[AlexeyMz]

Should Kefir.Stream emit a error in case of thrown exception in .map()?

For example, the following code will fail with an uncaught exception:

function f(x) {
  if (x % 2 === 0) {
    throw Error();
  } else {
    return x;
  }
}
const stream = Kefir.sequentially(1000, [1, 2, 3]);
const mapped = stream.map(f);
mapped.log();

There is a non-obvious way to prevent this by wrapping map callback into try-catch inside flatMap:

function f(x) {
  if (x % 2 === 0) {
    throw Error();
  } else {
    return x;
  }
}
const stream = Kefir.sequentially(1000, [1, 2, 3]);
const mapped = stream.flatMap(function (value) {
  try { return Kefir.constant(f(value)); }
  catch (error) { return Kefir.constantError(error); }
});
mapped.log();

[rpominov]

I'm personally strongly against automatic exceptions catching in async abstractions.

We should distinguish two types of errors and handle them separately. First type is bugs — when something completely unexpected happens in the program, when we get "foo is not a function" while it supposed to always be a function. Second type is expected errors that we must handle — errors we know may happen depending on user input, network condition etc.

The problem with automatic exceptions catching is that both types go into same handlers. This causes several issues like:

1. Handlers have to be either smarter and try to handle any error in a way that user always see correct program behavior (some errors need to be shown in a form, some should redirect user to another page, some should show a popup, etc.), or handlers can be very dumb (like always show a popup). The first option (being smarter) is way harder when we can get anything as a error in the handler.
2. We have to add error handlers to every stream, otherwise exceptions will be just swallowed which is the worse scenario.
3. Also we can't actually write code that handles bugs in a correct way. It's impossible. After a bug occurred we have no idea what have happened, we can't write a code that moves program from any random inconsistent state to a consistent one. The only way we can transition program to a consistent state after a bug is to restart it. Any code that tries to handle bugs makes it only worse: it makes bugs harder to debug and fix, it may result in turning database into inconsistent state as well, etc. IMO a better approach is to setup a reporting system (Sentry etc.) and to not try to handle bugs at all, just let program crash.

[AlexeyMz]

I agree with you about "the right" way to handle errors in theory. It is successfully implemented and used in several functional languages and even imperative ones like Rust.

The problem lies within the ugly reality of JS history, ECMA standards and libraries. For JS and some other languages (e.g. C#, Scala, ...) exceptions broadly used not only for "bugs" like TypeError but also for failed requests, invalid user input, etc.

Let me address the issues:

1. In practice, the most important about exceptions is not to catch them but to maintain the correct state. In current implementation it is very tedious to protect yourself against random exception on failed request added in a new version of some library.
2. It could be done without swallowing exceptions by logging them to console if there is no registered error handler. For example, default Promise implementation behaves this way.
3. See 1. At least, with exception handling there will be a way to write code to maintain consistent state. Currently it is almost impossible without manually adding try-catch in a flatMap for every projection operation.

Also, the current behavior is very different from almost any other "observable" (e.g. Rx) or "future" (.NET's Task, Scala's Future, etc) implementation which could be surprising for many users. See Rx Error Handling for example.

[rpominov]

At least, with exception handling there will be a way to write code to maintain consistent state.

I believe that it's impossible to maintain consistent state after a random unexpected exception. And also that putting random exceptions into failure path of Observable ruins "Railway oriented programming" use case.

I've summarized all my thoughts in an article recently: https://github.com/rpominov/fun-task/blob/master/docs/exceptions.md

the current behavior is very different from almost any other "observable" (e.g. Rx)

I'm trying to convince people that it's wrong there as well: https://github.com/tc39/proposal-observable/issues/104

[AlexeyMz]

And also that putting random exceptions into failure path of Observable ruins "Railway oriented programming" use case.

Does it? Any exception will be propagated to parent observable, you could still apply projection and join them.

I summarized all my thoughts in an article recently: https://github.com/rpominov/fun-task/blob/master/docs/exceptions.md

Some of the points there are not applicable anymore (exceptions never swallowed by Promise unless explicitly handled by .catch()), some others support my point. For the ideal world where JS is a purely functional language without exceptions you totally right, but it is better to have consistency with existing and future APIs.

I'm trying to convince people that it's wrong there as well: tc39/proposal-observable#104

They seem to have the same thoughts as me: inconsistencies with other implementations.

[rpominov]

Does it? Any exception will be propagated to parent observable, you could still apply projection and join them.

Yeah. The problem is that it is harder to write sane code when both expected failures and random exceptions go into the same callback.

It is basically the same situation as if if statement worked like that:

if (expression()) {
  // the `true` case goes here
} else {
  // the `false` case goes here
  // but also any error thrown from expression() goes here
}

For the ideal world where JS is a purely functional language without exceptions

I don't actually think that exceptions itself are bad in an impure language like JS. I think they are quite good for debugging. I just think that we should not: 1) catch them randomly 2) put them into the same callback that is intended for expected failures path. First ruins debugging experience because even to re-throw isn't the same thing than not catch in first place from debugging point of view. Second ruins railway use case (see continuation of if example here where I show how hard it would be to use if statement if it worked like that)

[AlexeyMz]

Yeah. The problem is that it is harder to write sane code when both expected failures and random exceptions go into the same callback. It is basically the same situation as if if statement worked like that:

You seem to imply that two types of exceptions is different enough to distinguish them when catching. In practice, you almost never care about catching exceptions, because you use try-finally equivalent instead and only catch errors at the top.

2) put them into the same callback that is intended for expected failures path

I would say that in many cases bug is a failure too. For UI applications it is common to show "generic error dialog" or something like that even in case of bug, because it is still better than having unresponsive UI.

First ruins debugging experience because even to re-throw isn't the same thing than not catch in first place from debugging point of view

Why rethrow is different from not catching? It should preserve stack trace.

[rpominov]

You seem to imply that two types of exceptions is different enough to distinguish them when catching.

We probably have in mind different examples. I think they are very different.

Why rethrow is different from not catching? It should preserve stack trace.

Debugger won't pause on the original throw line for instance. But it's hard to even re-throw (see second part of if example).

[niahoo]

Hi, I agree with you @rpominov , but I would like to know a way to manually pipe exceptions into the current stream, to be able to handle them with stream.onError. Is that possible ?

Thanks

[rpominov]

Could you tell more about your use case, @niahoo?

[niahoo]

Sure. I'm building a small game with an immutable state. The whole world is updated in a .scan() that receive functions and apply them to its state.

After the .scan there is a .onValue call where I pass the state to a react View. I would like to be able to update this view (or another view) in .onError. To do so, I would like to be able to catch exceptions and direct them to the error stream.

edit: I think I miss the point of errors in Kefir. I'll just send my errors as values to a stream of UI events to notify them to the player.

[rpominov]

I'm afraid there is no good way to catch in scan if you want to then redirect error to Kefir's error path. This would be possible with map, you could use flatMap instead:

stream.flatMap(x => {
 try {
   return Kefir.constant(unsafe(x))
 } catch(e) {
   return Kefir.constantError(e)
 }
})

Still in scan you can catch and put errors into main Kefir's path.

edit: I think I miss the point of errors in Kefir. I'll just send my errors as values to a stream of UI events to notify them to the player.

They basically model built-in Either.

[niahoo]

Well, in scan I must return a state, so I can't return the exception. But I send it to another stream using the emitter.

Thanks for the docs. btw the "How exceptions catching work in Task" link on fun-task home README is dead !

[rpominov]

I mean your state could be {data: ..., error: ...} for example.

stream.scan((state, fn) => {
  try {
    return {data: fn(state.data), error: null}
  } catch (e) {
    return {data: state.data, error: e}
  }
}, {data: initialData, error: null})

You can even put errors in the errors path then:

stream.scan(...).flatMap(state => state.error ? Kefir.constantError(state.error) : Kefir.constant(state.data))

Thanks for the warning about the links!

[niahoo]

Ok, thank you, it works great :)

[mAAdhaTTah]

Since this is foundational to the way Kefir was designed, this is unlikely to change, so I'm going to close this issue.