cujojs / most

Ultra-high performance reactive programming
MIT License
3.49k stars 206 forks source link

Continue stream after error #231

Open standy opened 8 years ago

standy commented 8 years ago

I wonder if there is a way to handle single error in the stream and continue? The only way i found is to make concatMap + recoverWith + most.empty()

stream$
    .concatMap(x => most.just(x)
        .map(mayThrow)
        .recoverWith(e => {
            console.log('CAUGHT', e);
            return most.empty();
        })
    )
    .forEach(x => console.log('result', x));

Would be nice to have method like handleError to do side effects and move on Or maybe tapErrors and filterErrors similar to tap and filter Or even concatMapError to replace each error with a stream

stream$
    .map(mayThrow)
    .tapErrors(e => console.log('CAUGHT', e))
    .filterErrors(e => e.message !== 'Known error')
    .forEach(x => console.log('result', x));
briancavalier commented 8 years ago

Hey @standy. Here's a bit of background on the meaning of errors in most.js. Basically, errors mean stream failure.

That filterErrors idea is interesting. I'll give it some thought ... I'm a little wary of giving people an easy way to silence errors, since it can hide bugs. Maybe naming it something like ignoreErrors or unsafelyIgnoreErrors would help.

tapError is pretty easy to do:

import { recoverWith, throwError } from 'most'

const tapError = (f, stream) => recoverWith(e => {
  f(e)
  return throwError(e)
}, stream)
briancavalier commented 8 years ago

@standy I've been thinking that the right thing here may be to augment recoverWith (or provide a variant of it) to allow catching only errors that match a predicate. It'd work like typed catch statements from other languages (some JS promise implementations, namely when.js and bluebird, provide this same feature for promises).

Any thoughts on that?

standy commented 8 years ago

Main problem i have met about errors is that stream is ending right after. All I can do is continue with another stream Cant do something like that with most.js (but can with any other frp library)

expected:  1-2-3-e-5-...
actual:    1-2-3-e|

To be less abstract, example: I want to make a method, which take a stream of questions, and return a stream of answers (just fetch from wikipedia). So I want method like that: getAnswers(questions$). But fetch can throw, so i need to handle error inside this method, that mean signature must be like that getAnswers$(questions$, onError)

briancavalier commented 8 years ago

fetch can throw, so i need to handle error inside this method, that mean signature must be like that getAnswers$(questions$, onError)

Ok, that helps, thanks. Assuming your fetched answers are promises, it seems like you can handle the error at the promise and join/chain/concatMap the errors away. For example, here's one way with one tiny helpers, using join. Note that interestingly, the contract of handleError is the same as you'd pass to recoverWith, so little to no additional cognitive load.

import { from, empty, just, throwError } from '../most'

// Return promise for either just(x), empty(), or throwError
// The contract of handleError is the same as you'd pass to recoverWith:
// handleError :: Error -> Stream
const handlePromiseErrors = handleError => promise =>
  promise.then(just, handleError)

const fetchAnswer = question => /* return a promise which might reject */

const getAnswers = (questions$, onError) =>
  questions$
    .map(fetchAnswer)
    .map(handlePromiseErrors(onError))
    .await()
    .join()

// An example of one way to implement handleError
const myHandleError = e =>
  isSkippableError(e) ? empty() : throwError(e)

Would that work for you?

standy commented 8 years ago

I would like to avoid an onError callback at all, and deal with errors same declarative way as with values, like

getAnswers(questions$)
  .tapErrors(logError)
  .filterErrors(isSkippableError)

OnError callback actually do the side effect (log somewhere), filter errors, and return the stream at same time. Looks dirty (no offence, your code is great :)) Like if you would use onValue callback instead of returning stream.

standy commented 8 years ago

Check out this talk by Scott Wlaschin: https://youtu.be/E8I19uA-wGY?t=42m52s (about error handling from 42m) Best approach I've ever seen

standy commented 8 years ago

Got a better example to demonstrate what i mean On each question to get answers (maybe more then one), and check each answer (result is checked answer or empty). If error happens it should be handled and process should not be stoped. Some code there: http://requirebin.com/?gist=a7a25b95abcacaf3d09a97e299b09d6d

So main code without error handling looks like:

questions$
  .flatMap(getAnswers) 
  .flatMap(checkAnswer)

But if i want to add error handling i should add onError callbacks and rewrite my methods (which is not always trivial for async code). With error handling this app will become more complex then it should be.

On another hand there is approach like promises do:

promise
  .then(...)
  .then(...)
  .catch(...)

If we had .catch in most.js, error handling would be easy one-liner. Existing recoverWith method is similar, with one exception — it ends current stream, so its doesent fit in some cases.

briancavalier commented 8 years ago

Hey @standy sorry I haven't a had a chance to respond to all the good stuff you've been posting. I've been really busy (including trying to get ready for most.js 1.0). I'll try to respond soon!

kaosat-dev commented 7 years ago

Hi @standy I am struggling with pretty much exactly the same issues , how (if) did you solve it in the end ?

standy commented 7 years ago

Hi @kaosat-dev, I did with onError callback, but then did roll back to promises and async/await. Maybe another solution would be catch in ajax promise, and make stream with mixed type: value or Error, where you could filter either stream.

stream$
    .map(valueOrError)
    .filter(x => !(x instanceof Error))
    .forEach(x => console.log('value:', x));

I suppose better to avoid exeptions at all, so should check every ajax response and external inputs anyway

kaosat-dev commented 7 years ago

thanks for the infos @standy ! So if you use async await, did you ditch the streams part completely ?

On my side, for now I ended with handling errors 'locally' within a flatMap + internal flatMapError, not the best, but works

.flatMap(function (id) {
      return printerCamera(id)//this returns a Fetch call wrapped in a Most.js observable
        .flatMapError(error => of(undefined))//or do side effects
        .filter(x => x !== undefined)
    })

there is also the solution of using a more 'imperative' wrapper, with two Subjects that would recieve sucess data or errors respectively , but that is kindof a dirty solution