Skip to content

Commit

Permalink
Fix shared state in retry (#829)
Browse files Browse the repository at this point in the history
* Fix retry using shared state

* update changelog

* Update CHANGELOG.md

* Add unit test

Co-authored-by: Anders Ha <anders@andersha.com>
  • Loading branch information
sebastiangrail and andersio committed Nov 19, 2021
1 parent 2ab421a commit 7cbe50f
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# master
*Please add new entries at the top.*

1. Fixed issue where `SingalProducer.try(upTo:interval:count:)` shares state between invocation of `start` on the same producer.

# 6.7.0
# 6.7.0-rc1

Expand Down Expand Up @@ -42,7 +44,6 @@
1. Bumped deployment target to iOS 9.0, per Xcode 12 warnings. (#818, kudos to @harleyjcooper)

1. Fixed a few deprecation warning when the project is being built. (#819, kudos to @apps4everyone)
>>>>>>> origin/master

# 6.5.0

Expand Down
7 changes: 5 additions & 2 deletions Sources/SignalProducer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2393,9 +2393,10 @@ extension SignalProducer {
return producer
}

var retries = count
return SignalProducer { observer, lifetime in
var retries = count

return flatMapError { error -> SignalProducer<Value, Error> in
lifetime += flatMapError { error -> SignalProducer<Value, Error> in
// The final attempt shouldn't defer the error if it fails
var producer = SignalProducer<Value, Error>(error: error)
if retries > 0 {
Expand All @@ -2408,6 +2409,8 @@ extension SignalProducer {
return producer
}
.retry(upTo: count)
.start(observer)
}
}

/// Wait for completion of `self`, *then* forward all events from
Expand Down
34 changes: 34 additions & 0 deletions Tests/ReactiveSwiftTests/SignalProducerSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2812,6 +2812,40 @@ class SignalProducerSpec: QuickSpec {
expect(errors) == [.default]
}

it("should not share retry counter across produced signals") {
let scheduler = TestScheduler()
var failuresReceived = 0

let original = SignalProducer<Int, TestError> { observer, _ in
scheduler.schedule { observer.send(error: .default) }
}
let retryDelay: DispatchTimeInterval = .seconds(1)
let retriable = original
.retry(upTo: 1, interval: retryDelay.timeInterval, on: scheduler)
.on(failed: { _ in failuresReceived += 1 })

// 1st produced signal
retriable.start()

scheduler.advance()
expect(failuresReceived) == 0

scheduler.advance(by: retryDelay)
expect(failuresReceived) == 1

// 2nd produced signal
retriable.start()

scheduler.advance()
// Shared retry counter had caused the second produced signal not to defer the failed event as expected.
// (https://github.com/ReactiveCocoa/ReactiveSwift/pull/829)
// Assert that we did not receive the 2th final failure at this point, which should always be delayed until
// at least `retryDelay` has elapsed.
expect(failuresReceived) == 1

scheduler.advance(by: retryDelay)
expect(failuresReceived) == 2
}
}

}
Expand Down

0 comments on commit 7cbe50f

Please sign in to comment.