coinbase / temporal-ruby

Ruby SDK for Temporal
Apache License 2.0
213 stars 81 forks source link

Cannot handle signals on first workflow task #267

Closed jeffschoner closed 8 months ago

jeffschoner commented 8 months ago

Signals received before the first workflow task are effectively not available to code running in the first workflow task. Notably, this happens when using start-with-signal.

This happens because since #261 events within the first workflow task are handled in the following order:

  1. Markers for versions and side effects
  2. Start workflow execution. This is always the first history entry in every workflow run. This causes workflow code to start executing.
  3. Signals

Although 'start workflow execution' starts workflow code which can register a signal handler, no signals are invoked against those handlers until after the workflow code deschedules, such as when encountering a sleep or waiting for an activity to complete.

For example, consider the following code:

class ReproWorkflow < Temporal::Workflow
  def execute
    received = nil
    workflow.on_signal do |signal, input|
      received = input
    end

    if received.present?
      'got signal'
    else
      'no signal'
    end
end

# Start this with a signal
Temporal.start_workflow(ReproWorkflow, {signal_name: 'foo', signal_input: 'bar'})

The workflow started here will always return 'no signal' because received will not be updated until after the workflow function has already returned.

This problem does not exist outside of the first workflow task because the signal handler can simply be registered earlier in the workflow. It's not possible to pre-register a signal handler before execution.

Workarounds include:

Neither of these are great solutions, but they do limit the severity of this bug.

jeffschoner commented 8 months ago

I have a fix for this mostly ready to go. We've been running it inside of Stripe for a while with no issues. I should have a PR up soon.