Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

previous request are cancelled by the following request #36

Open
dqw18037 opened this issue Aug 5, 2018 · 3 comments
Open

previous request are cancelled by the following request #36

dqw18037 opened this issue Aug 5, 2018 · 3 comments

Comments

@dqw18037
Copy link

dqw18037 commented Aug 5, 2018

I'm using the RxFeedback in my project, and i got a problem that a processing network request will be cancelled by the follwing request.

To describe the question clearly, i made a demo here.

the UI is below, it's simple

screen shot 2018-08-05 at 6 06 32 pm

If 'request 1' button taped, a simulative request will be send, and delay to 2.0s, the button below will show 'response 1 received'.
and if 'request 2' button taped, 'response 2 received' will display the same way.

While, the problem is that if 'request 1' button taped, and before response 1 received, i taped the 'request 2' button, then the response 1 will never come back, it is cancelled.

And if i make debug here:
let event = just.delay(2.0) .debug(query.debugIdentifier, trimOutput: false)
i will got the following log:

---- request 1 ----
-> subscribed
---- request 2 ----
-> subscribed
---- request 1 ----
-> isDisposed
---- request 2 ----
-> Event next(response(RxFeedbackQueryTest.RequestType.second))
---- request 2 ----
-> Event completed
---- request 2 ----
-> isDisposed

we can see that request 1 was disposed once request 2 is subscribed

after reading the source codes of RxFeedback, i think, the codes leading the problem is below:

1

2

3

by using 'flatMapLatest' and 'switchLatest' operators, the previous sequence will be disposed,
why did you design to cancel the previous 'query' if a new 'query' comes?
can i just replace the 'flatMapLatest' with 'flatMap', and remove the operation 'takeUntilWithCompletedAsync'?

and how 'takeUntilWithCompletedAsync' can avoid the reentrancy issues?

The main code is below:
` private func setupBinding() {

    let UIBindings: (Driver<State>) -> Signal<Event> = bind(self) { me, state in
        let subscriptions = [
            state.map { $0.response1 }.drive(me.responseLabel1.rx.text),
            state.map { $0.response2 }.drive(me.responseLabel2.rx.text)
        ]
        
        let events: [Signal<Event>] = [
            me.requestButton1.rx.tap.map { Event.request(.first) }.asSignal(onErrorJustReturn: .none),
            me.requestButton2.rx.tap.map { Event.request(.second) }.asSignal(onErrorJustReturn: .none),
            me.clearButton.rx.tap.map { Event.clear }.asSignal(onErrorJustReturn: .none)
        ]
        return Bindings(subscriptions: subscriptions, events: events)
    }
    
    let nonUIBindings: (Driver<State>) -> Signal<Event> = react(query: { (state) -> Set<RequestType> in
        let querys = Set(state.requestsToSend.all())
        return querys
    }) { (query) -> Signal<Event> in
        let just = Signal.just(Event.response(query))
        let event = just.delay(2.0)
                     .debug(query.debugIdentifier, trimOutput: false)
        return event
    }
    
    Driver
        .system(initialState: State(),
                reduce: State.reduce,
                feedback: UIBindings, nonUIBindings)
        .drive()
        .disposed(by: bag)
}`

`enum Event {
case none
case request(RequestType)
case response(RequestType)
case clear
}

struct State: Mutable {

var requestsToSend = Requests()

var response1: String?
var response2: String?

static func reduce(state: State, event: Event) -> State {
    switch event {
    case .request(let req):
       return state.mutateOne {
            $0.requestsToSend.append(req)
        }
    case .response(let req):
        return state.mutateOne {
            switch req {
            case .first:
                $0.response1 = req.responseString
            case .second:
                $0.response2 = req.responseString
            }
        }
    case .clear:
        return state.mutateOne {
            $0.response1 = nil
            $0.response2 = nil
            _ = $0.requestsToSend.all()
        }
    default: return state
    }
}

}`

@kzaher
Copy link
Member

kzaher commented Aug 5, 2018

Use a set overload of feedback loop or create your own feedback loop.

@dqw18037
Copy link
Author

dqw18037 commented Aug 5, 2018

@kzaher i updated the question, and the demo address was set.
i hope you can run the demo to make sure you know my question correctly
actually, i didn't understand your reply above
does 'a set overload of feedback loop' mean the 'Set<Query>',
and what do you mean 'create my own feedback loop',
does it means that i can redefine the ' react<State, Query, Event>' function in 'Feedbacks' file?

Thx

@eliekarouz
Copy link

eliekarouz commented Sep 22, 2018

@dqw18037

The problem is a misusage of the library. You are using Requests which is a reference type as a storage of the requests but IN the query this line state.requestsToSend.all() is mutating the shared storage.
So the next time the query is computed, the previous requests that already fired an effect are not there anymore and their corresponding effects are canceled.

If the above doesn't make sense to you, then think of how you would it if you have only one request.
You would have to encode an optional variable of RequestType in the state and when the optional request becomes != nil then you fire the effect. You return the value back to nil is when you get the response. If you return it back to nil before you receive the response, then no event will be emitted.

Instead of the Requests class just use the Set in the State and keep the RequestType in the Set until you get a response. In case you are not interested in the result of some RequestType remove it from the Set and the flatMapLatest will make sure you don't get any event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants