repeatWhen completes() subscriber immediately when notifier completes()

[davidwdan]

RxJS version: 5.0.0-rc.1

Code to reproduce:

Rx.Observable.interval(500)
    .take(3)
    .repeatWhen(attempts => attempts.delay(1000).take(1))
    .subscribe(
        x => console.log('Next: %s', x),
        err => console.log('Error: %s', err),
        () => console.log('Completed')
    );

Expected behavior:

Next: 0
Next: 1
Next: 2
//waits 1 second
Next: 0
Next: 1
Next: 2
Completed

Actual behavior:

Next: 0
Next: 1
Next: 2
//waits 1 second
Completed

Additional information:
RxJS 4 matches the expected behavior

[jayphelps]

Confirmed. This behavior differs from v4, not immediately clear whether that's from intentional changes or a bug.

Here's a demo comparing v4 to v5: http://jsbin.com/zehuqo/edit?html,js,output

While I certainly could be wrong, it appears the difference in implementation is that v5 will complete() the subscriber right when the notifier observable completes, even if the repeated source was in the middle of scheduling something. Whereas when v4 retryWhen receives a complete() from the notifier, it seems to let the repeated source complete() gracefully but try to repeat it again.

So in the presented example what is happening is that your notifier says to take(1), which means it will resubscribe to the source, then immediately complete the downstream subscriber. Meaning it repeats the source synchronously and then also synchronously completes the downstream subscriber. So since the topmost source is an interval, there's nothing for it to emit in time, because it schedules for 1000 in the future. However, if the source emits things synchronously (no async scheduling) then indeed it will emit them before retryWhen gets a chance to complete()

What I'm talking about can be demo'd by sources which do not schedule async:

http://jsbin.com/caxiso/edit?html,js,output

Observable.of(1, 2, 3)
  .repeatWhen(attempts =>
    attempts
      .delay(1000)
      .take(1)
  )
  .subscribe(
    x => console.log('Next: %s', x),
    err => console.log('Error: %s', err),
    () => {
      console.log('Completed');
    }
  );

Next: 1
Next: 2
Next: 3
//waits 1 second
Next: 1
Next: 2
Next: 3
Completed

(both v4 and v5 produce the same behavior for that demo)

Which behavior is "right" and expected? I can see people wanting either, depending on use cases. Either behavior we choose it probably should be documented more clearly. My gut tells me the v4 behavior is more intuitive for the most common cases.

[jayphelps]

@slavugan your code logs complete because you're not actually passing a function, you're calling console.log immediately. Looks like a typo.

[jayphelps]

@Blesh has volunteered to dig into this. 👍

[mattpodwysocki]

@jayphelps the correct behavior is v4 as it conforms directly to the Java implementation which is the original implementation, so yes, this is a bug.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.