statelyai / xstate

Actor-based state management & orchestration for complex app logic.
https://stately.ai/docs
MIT License
27.1k stars 1.25k forks source link

Sending an event to 'false' freezes child machines (and maybe xstate) #1872

Closed ratacat closed 1 year ago

ratacat commented 3 years ago

Description I have an interpreted machine, with a child machine, child machine is awaiting for a large number of async functions to resolve. In the middle of that, if I send an event to false from parent machine, the child machine freezes, and maybe the parent too?

    actions: {
      relayGasUpdate: send(
        { type: 'GAS_UPDATED1', data: (context, event) => event },
        {
          to: (context, event) => {
            return context.handler1 //this is resolving to false
          },
        },
      ),

Sorry, I don't have a reproduction. I'll see if I can play with it when I have a moment.

Expected Result Would expect no effect, or maybe an error

Actual Result no feedback, everything just freezes

Reproduction

Additional context xstate v4.14.0

Andarist commented 3 years ago

Please always try to share a repro case in a runnable form - either by providing a git repository to clone or a codesandbox. OSS maintainers usually can't afford the time to set up the repro, even if exact steps are given.

ratacat commented 3 years ago

@andarist, I'd love to thanks! Haven't had a moment yet, but its on my list.

davidkpiano commented 3 years ago

@ratacat Curious, what's the expected behavior here? Should XState throw an error if trying to send to a falsy actor, or should it do nothing?

Also, I spy blockchain!

ratacat commented 3 years ago

@davidkpiano I would expect an error in trying to send to a falsy actor, but if nothing happened that wouldn't be a huge deal either. The big deal for me was actually that it froze multiple of my machines best I can tell, like they totally just stopped, but didn't even throw errors. I am kind of in blinders mode, in that the system I am working on has a limited lifespan right now...basically when Uniswap upgrades to v3, it's done. But this is definitely on my list to circle back and make a good proof of concept for this bug. I REALLY REALLY appreciate everything you've done with xstate, and the phenomenal level of support you've provided around it.

Yeah, I've been making a bot that runs on the Ethereum network with xstate. It's been hugely helpful, and learning to use a state machine has been on my life's learning goal list for a while now =)

davidkpiano commented 3 years ago

cc. @Andarist - seems like this is a good use-case for error.communication (SCXML):

// will raise 'error.communication'
actions: send('SOME_EVENT', { to: false })

These can be caught:

on: {
  'error.communication': {/* ... */}
}
Andarist commented 3 years ago

Actually, this one should result in error.execution as it is invalid and not just non-existent: https://www.w3.org/Voice/2013/scxml-irp/194/test194.txml versus: https://www.w3.org/Voice/2013/scxml-irp/521/test521.txml https://www.w3.org/Voice/2013/scxml-irp/496/test496.txml

davidkpiano commented 1 year ago

Closing as this needs a reproduction, and this might already be fixed in xstate@beta; please try that out 🙏