kefirjs / kefir

A Reactive Programming library for JavaScript
https://kefirjs.github.io/kefir/
MIT License
1.87k stars 97 forks source link

Errors #15

Closed DNizovtsev closed 9 years ago

DNizovtsev commented 9 years ago

For first thanks for nice library. I want to use it instead of bacon in my project. But library does not support errors handling right now. If you do not mind I can add errors handling like bacon: Error object, onError, errors, skipErrors, retry methods for Observables.

Any thoughts?

rpominov commented 9 years ago

Thanks, but I think It will be pretty big code update, so I'll probably do it myself.

By the way, what do you need error handling support for? I am personally use Kefir only in the browser, and in the browser the only natural source of errors I can think of is network operations, anything else is errors free. For example you don't expect errors from stream representing mouse clicks. So for me Kefir is good without errors support as long as I use it in the browser and not for ajax. That is the reason why I didn't add errors so far.

So what is your use case? You want to use Kefir on server side, or you have a lot of potential error sources in the browser?

DNizovtsev commented 9 years ago

You are right, I want to try use FRP on the server side, and it will hard without error handling

rpominov commented 9 years ago

I am planning to add errors support eventually, but can't tell for sure when it will happen. Now Kefir probably not good for tasks where you expect errors and I'd recommend to use Bacon in that case.

DNizovtsev commented 9 years ago

ok, thanks

tehsenaus commented 9 years ago

+1 for this:

Though DOM events don't generate errors, any subsequent processing you do downstream might. IMO the lack of error handling severely limits the scope of the library.

rpominov commented 9 years ago

Though DOM events don't generate errors, any subsequent processing you do downstream might.

Could you give some example?

tehsenaus commented 9 years ago

Validation of user inputs is a simple one (though I want to use Kefir for server communication too).

The performance gains of your library vs Bacon.js are impressive, but without error handling I might have to switch back :(

rpominov commented 9 years ago

Thanks! I'll work on this as soon as I will have enough free time (probably in January).

chicoxyzzy commented 9 years ago

My team is in search for blazing fast FRP library for use in our performance critical project. Missing error handling stop us from using Kefir together with Bluebird promise library (so in fact we need #10 too). Looks like most voted feature here!

massimiliano-mantione commented 9 years ago

:+1: I also like this library, but error handling would be really needed (and not just in nodejs) :smile:

rarous commented 9 years ago

Guy, you can send "errors" as different kind of data through the stream. You don't need another channel for errors. It is convenient for strongly typed languages, but JavaScript is dynamic. Keep it simple.

massimiliano-mantione commented 9 years ago

True, and in fact reducers passes errors as Error objects.

The point is that all the reducers modules are consistent in this, so one knows what to expect from a third-party stream of events (what to consider data, and what to consider an error). Errors, like exceptions, are a fact of life, even nodejs callbacks understand this (and in fact #21 is blocked by this issue).

Moreover, is would be nice if the "reactive framework" (in this case Kefir, otherwise RxJs, Bacon, Highland...) wrapped every callback invocation in a try-catch, and emitted the eventual exception as an error instead of as "data".

RxJs and Highland both do it (I did not read the code of Bacon and Reducers). This way you can just pass any callback (which can in turn call anything) and know that there will be no uncaught exceptions: every error will be clearly reported on the "event stream".

jasonkuhrt commented 9 years ago

In order to help @pozadi prioritize his work on this library I will add that at littleBits we are very interested in Kefir but no error handling or ad-hoc error handling as suggested by @rarous is not acceptable for us. Our use-cases span client and server, where we view the definition of a modern system to imply sufficiently evented to warrant FRP as a general strategy for dealing with all manner of temporal complications.

rpominov commented 9 years ago

It already next task in my todo. Going to start at this weekend, and hope to finish the work by mid-January.

jasonkuhrt commented 9 years ago

@pozadi I have no direct advice to share on this issue but maybe you can get some useful insight about discussion regarding this topic from the Elm community, see this thread: https://groups.google.com/forum/?fromgroups#!searchin/elm-discuss/signal$20error/elm-discuss/DiVqj6BsFtU/R1sklUTVRVEJ

Clearly the issue is quite complicated and loaded. Would you be willing to share some preview releases (on a branch?) that demonstrate API ideas for errors? I don't think its a given that .onError is the best option here.

rpominov commented 9 years ago

I was going to implement it similar to Bacon (with .onError etc.) Already have a pretty clear picture in my mind of how it will work.

I'll look into discussion you mentioned, but I don't feel very optimistic of trying any fresh approach at this point, to be honest.

If you have any ideas of better implementation, please share.

jasonkuhrt commented 9 years ago

@pozadi Still don't have time to get involved yet but it might be interesting to expose an ioStream type whose values are all Either types.

jasonkuhrt commented 9 years ago

@pozadi with .onError and friends composition is inherently complicated by the fact that errors and values are widely separated when in reality they are cohesive. If anything .onError could just be sugar for filtering for Either values with a left value (whereas the right value in Either is the "OK" value).

rpominov commented 9 years ago

@jasonkuhrt I don't think errors and values will be separated with current (Bacon.js like) approach. You'll be able to handle them both in same callback using .onAny() and .withHandler() methods. And it will be easy to create other methods that works with both errors and values on top of .withHandler().

Also it's happening: https://github.com/pozadi/kefir/pull/28 :)

rpominov commented 9 years ago

I have finished base errors support in a branch https://github.com/pozadi/kefir/pull/28 (see a little more detailed description of what been done in the PR comment). Next step is to add helper methods here and there, like .mapErrors etc.

rpominov commented 9 years ago

v0.5.0 released with errors support. See changelog https://github.com/pozadi/kefir/blob/master/changelog.md#050

rpominov commented 9 years ago

See also https://pozadi.github.io/kefir/#about-errors

chicoxyzzy commented 9 years ago

:+1:

gnapse commented 9 years ago

One of the features mentioned in this issue's original text is the retry capability, as provided in Bacon and RxJS. Is this covered in these recent additions? Because at first sight it would appear it's not. And if that's the case, are there any plans on adding support to it? or perhaps an easy way to achieve it by composing current methods?

rpominov commented 9 years ago

You can do retry using repeat method. Something like this:

var ajaxResult = Kefir.repeat(function(i) {
  if (i < 2) {
    // on first two tries ignore errors
    return ajaxCall().skipErrors(); 
  } else if (i === 2) {
    // on the third try let the error (if there will be a error) show up in `ajaxResult`
    return ajaxCall(); 
  } else {
    // on the fourth iteration just return `false` to stop the cycle
    return false; 
  }
}).take(1); 
// without `.take` it will make three ajax calls even if all of them was success

Maybe we need a convenient .retry method that will basically do the logic above. But for now you can use .repeat.

gnapse commented 9 years ago

Thanks, that'll do the trick, but +1 for that convenience method .retry. It'll definitely be a good addition.

Thanks for the library by the way. Its paradigm is much better than that of RxJS (errors do not end streams, and the avoidance of synchronous observables). Plus it's way faster than Bacon.js. That makes up for the lack of some convenience methods like this one.

maple-leaf commented 8 years ago

The repeat trick is not as clear as a retry method, votes for adding a retry method. And thanks for the great job.

rpominov commented 8 years ago

If I understand correctly how retry works, it won't work with "hot" observables (observables that have many subscribers connected to single original source of events). And in Kefir we have only "hot" observables.

maple-leaf commented 8 years ago

I think bacon.js is also hot only, take a look at its api ref, https://baconjs.github.io/api.html, scroll to the end, and you will see this description at the For RxJs Users section: Also, there are no "cold observables", which means also that all EventStreams and. So, I think maybe retry can work with hot.

rpominov commented 8 years ago

I mean if we do retry in Kefir, it will be not like in Rx obs.retry() but rather:

Kefir.retry(() => { 
  // This function must return a not ended `obs` that's guaranteed 
  // to not have any subscribers at the moment.
  // Ideally just create a fresh observable from a scratch.
  return obs
})

In example above ajaxCall is such a function.


Edit: added "a not ended".

rpominov commented 8 years ago

Yeah. Notice that in Bacon API looks like this Bacon.retry({source: ajaxCall}). Given the restrictions on what ajaxCall must return, I think it will be confusing and bug prone for people to use such retry.