rxjs: Can't repeat or retry Observable.bindCallback and Observable.bindNodeCallback

Observable.bindCallback and Observable.bindNodeCallback create internal AsyncSubjects that live forever after the first subscription, which means we can’t retry or repeat these Observables.

I understand this is how it worked before, but that behavior seems to be incidentally caused by the old multicast behavior disallowing retrying and repeating. Does anybody else think we should allow retrying/repeating, or at least allow end users to override the multicast behavior?

import Rx from 'rxjs';

const getObs = Rx.Observable.bindCallback(
  (msg, cb) => {
    console.log('trying callback');
    cb(new Error(msg));
  },
  (error) => { throw error; }
);

// only prints `trying callback` once when expected to print 3 times

getObs('some error')
  .retry(3)
  .subscribe(null, (finalError) => {
    console.log("got error:", finalError);
  });

About this issue

Original URL
State: closed
Created 8 years ago
Reactions: 3
Comments: 24 (10 by maintainers)

Most upvoted comments

I too find it very odd that calling subscribe() on the Observable created by bindCallback() does not re-trigger the callback. This behavior completely breaks otherwise normal repeat/retry logic in the midst of a larger Observable sequence. I always have to remember to wrap the bindCallback() in defer() or something similar to get it to trigger again – which seems unintuitive and is often forgotten.

Just like with any “normal” Observable, the default expectation should be that calling subscribe() will re-do the work. IMO, that would be the Least Surprising thing to happen.

If one desires to share/cache work, then publish() or some other technique can be used (which would also then serve to explicitly document the intended behavior).

mattflix on Jan 24, 2017

I am also for bringing back fromCallback in a form that works “correctly” – as a simple, cold observable that does exactly what re-invoking the underlying callback API would do (i.e., does the work over, every time).

I would recommend renaming bindCallback to publishCallback to reduce confusion and set expectations about what it actually does. That said, there doesn’t seem to be any “need” for a publishCallback (other than as a convenience) since the same behavior could always be composed.

If it were me, I would just nix bind/publishCallback and just have a nice, cold fromCallback.

On related question… why was the context argument removed? The Rx4 version of fromCallback took an optional context parameter to be passed as the this pointer of the invoked callback API. With the current Rx5, I must first manually api.method.bind(api) instance method callback APIs before passing them to bindCallback. That itself (the two uses of the word “bind”, plus the need for both “bind” operations) could also be a source of unnecessary confusion.

mattflix on Jan 26, 2017

I’d really like to see this fixed.

mattkime on Sep 6, 2016

Is it possible to revisit this discussion, now that we’re in RxJS6?

larssn on May 25, 2018

@mattflix, @trxcllnt el al: There’s a couple things to discuss here:

a bindCallback observable can’t be retried/replayed.
a bindCallback observable only calls the callback once and caches the result.

I think 1 is an issue. We should be able to retry or replay them, or we should rename this to publishCallback, since publish variants can’t be retried or replayed.

2, on the other hand is stickier. This method was created to model Rx 4’s fromCallback… since the behavior was different from other fromX methods in that it made a hot observable that cached the result, we renamed it to bindCallback. I don’t think I want to change the behavior of this, BUT, I’m all for adding a fromCallback method that creates a cold observable, from which we can compose any behavior we want.

Thoughts?

benlesh on Jan 25, 2017