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

Cannot handle signals on first workflow task #267

Closed
jeffschoner opened this issue Oct 15, 2023 · 1 comment · Fixed by #268
Closed

Cannot handle signals on first workflow task #267

jeffschoner opened this issue Oct 15, 2023 · 1 comment · Fixed by #268

Comments

@jeffschoner
Copy link
Contributor

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:

  • Adding code that deschedules the workflow between signal handler registration and reading any of its effects, such as sleeping for 1 second before checking the value of received above
  • Accepting that signals will be delivered later in a workflow's execution than expected

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

@jeffschoner
Copy link
Contributor Author

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.

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

Successfully merging a pull request may close this issue.

1 participant