sideeffect-io / AsyncExtensions

AsyncExtensions aims to mimic Swift Combine operators for async sequences.
MIT License
324 stars 26 forks source link

[BUG] Latest Main branch commit breaks AsyncStreams (CurrentValue, Passthrough etc) #8

Closed Prince2k3 closed 2 years ago

Prince2k3 commented 2 years ago

Describe the bug I noticed a code behavior change since adding the actor code. For example, I use Passthrough as a way to pass events to an onReceive method. That stopped as it supposed the moment I updated to the updated main. The best I can describe the behavior is that fires once and then stops working after that.

To Reproduce Steps to reproduce the behavior:

var events = AsyncStreams.Passthrough<Event>()

// I used 
events.nonBlockingSend(.myEvent)

then


// I have a modifier that wraps this in SwiftUI (onReceive)
task {
 do {
         for try await event in viewModel.events {
               switch event {
                   ...
               }
         }
     } catch {}
}

Expected behavior AsyncStreams works like the commit e4aeb0f and before.

Environment:

Additional Info

I'm not sure how to really explain what is going on here. This is the best I can do.

twittemb commented 2 years ago

Hi @Prince2k3

I'm currently rollbacking from using Actors for the Passthrough/CurrentValue/Replay streams due to potentially losing some values while not being yet registered as a client of these streams.

Anyway, it could be nice if you could provide a repo that I could take a look at and make sure the next version will restore the expected behaviour.

thanks.

twittemb commented 2 years ago

@Prince2k3

could you check with that branch please ? -> https://github.com/AsyncCommunity/AsyncExtensions/tree/feature/multicast

for the record nonBlockingSend has been remove. you can use send directly.

Prince2k3 commented 2 years ago

@twittemb

I can definitely take a look this evening. This is definitely interesting using Multicast.

Can I ask why did actor not work in this case? I am also learning about the Async/ Await and Actor mechanism.

twittemb commented 2 years ago

There was an issue using an Actor to handle the registrations of new continuations in the streams (Passthrough, CurrentValue, Replay). The registration had to be Async since performed on an Actor. Unfortunately the creation of an AsyncStream offers a callback that does not supports Async. So the registrations had to be performed in a Task. In some occasions, some values could be pushed to the streams while this task was not yet executed and the continuation was also not registered yet. It resulted in loosing these values.

Prince2k3 commented 2 years ago

I reviewed the PR on the branch and used that branch to run through the usage in my current code. Seems to be working. I'll definitely be running more tests today.

@twittemb

twittemb commented 2 years ago

@Prince2k3 hey

did you had the chance to perform further tests ? should we close the issue ?

thanks.

Prince2k3 commented 2 years ago

Yes! It's working as intended. This can be closed.

twittemb commented 2 years ago

Thanks