dhiaayachi / temporal

Temporal service
https://docs.temporal.io
MIT License
0 stars 0 forks source link

Server shouldn't require every WFT to process all the signals #299

Open dhiaayachi opened 3 weeks ago

dhiaayachi commented 3 weeks ago

Such a requirement could be very inconvenient and could require in-compatible change in user workflow code.

Expected Behavior

Only WFT that closing workflow should be required to handle all signals.

Actual Behavior

It started to require at 1.21.4 version

Steps to Reproduce the Problem

In my PR: When changing the server version to 1.21.4 from 1.20.4 it fails https://github.com/indeedeng/iwf/pull/336/commits/6e858321b7af12d5be20cf4f9992213b6d333b23 with error in the logs:

2023-08-24T20:39:32.3249001Z 2023-08-24T20:39:32.324Z   DEBUG   loggerimpl/logger.go:117    received API request    {"value": "{\"signalChannelName\":\"test-unhandled-signal-name\",\"signalValue\":{\"data\":\"test-data-2\",\"encoding\":\"json\"},\"workflowId\":\"signal1692909569243681150\"}", "logging-call-at": "handler.go:66"}
2023-08-24T20:39:32.3346140Z 2023/08/24 20:39:32 INFO  Task processing failed with error Namespace default TaskQueue Interpreter_DEFAULT WorkerID 10311@fv-az1410-138@ WorkerType WorkflowWorker Error UnhandledCommand

Specifications

dhiaayachi commented 1 week ago

Thank you for reporting this issue.

It seems that you are experiencing an issue with Temporal 1.21.4 where your workflow is receiving signals that it is not expecting.

Based on the provided error message and the logs, it appears that the workflow is failing to handle a signal named "test-unhandled-signal-name". This suggests that the workflow definition does not have a corresponding signal handler for this signal.

To resolve this issue, you need to add a signal handler for the test-unhandled-signal-name signal to your workflow definition. This can be done using the SignalMethod annotation in the Signal method.

Here is an example of how to add a signal handler for the test-unhandled-signal-name signal:

import (
    "context"
    "github.com/temporalio/temporal/client"
    "github.com/temporalio/temporal/common/log"
    "github.com/temporalio/temporal/service/worker"
    "github.com/temporalio/temporal/workflow"
)

type MyWorkflow struct {
}

func (t *MyWorkflow) Signal(ctx workflow.Context, signalName string, input interface{}) error {
    switch signalName {
    case "test-unhandled-signal-name":
        // Handle the signal here
        log.Info(ctx, "Received test-unhandled-signal-name signal")
        return nil
    default:
        return workflow.ErrSignalNotSupported
    }
}

func main() {
    c, err := client.NewClient(client.Options{
        // ...
    })
    if err != nil {
        panic(err)
    }
    defer c.Close()

    w := worker.New(c, "MyWorkflowWorker", worker.Options{
        // ...
    })
    w.RegisterWorkflowWithOptions(MyWorkflow{}, worker.RegisterWorkflowOptions{
        Name: "MyWorkflow",
    })
    w.Run(context.Background())
}

For more information on how to handle signals in Temporal, please refer to the Temporal documentation: https://docs.temporal.io/docs/concepts/signals

dhiaayachi commented 1 week ago

Thank you for reporting this issue. It seems like you are encountering an issue with the Temporal server version 1.21.4 that requires all workflow types to handle signals, even if they are closing. We will investigate this issue further and update you as we progress.

In the meantime, to work around this issue, you can consider the following:

  1. Downgrade to a previous version: You can try downgrading to Temporal 1.20.4 and see if that resolves the issue.
  2. Handle the signal in the closing workflow: You can add code in your closing workflow to handle the signal, even though you might not need it. This would ensure that the workflow can handle all signals and avoid the error.

We are currently looking into the cause of this change in behavior between versions 1.20.4 and 1.21.4. Please follow the Temporal GitHub repository for updates on this issue.

dhiaayachi commented 1 week ago

Thank you for reporting this issue. It looks like you're experiencing an issue where the signal handler is not being invoked for the specified signal in Temporal 1.21.4. This appears to be a known issue.

You can find more information and workarounds about this issue in the Temporal documentation.

Temporal Documentation for Signal Handlers

Please note that the Temporal team is actively working on resolving this issue, and a fix will be available in a future release.

In the meantime, consider using the following workarounds:

Let us know if you have any further questions.

dhiaayachi commented 1 week ago

Thank you for reporting this issue.

This issue has been resolved in Temporal 1.22.0.
We found an issue where the signals received by a workflow were not correctly routed to the right workflow execution. This issue was corrected in Temporal 1.22.0. Please upgrade to Temporal 1.22.0 or later.

The issue you are encountering is due to a change in how Temporal handles signals in version 1.21.4. In previous versions, if a workflow execution received a signal that it was not able to handle, Temporal would ignore the signal. However, in version 1.21.4, Temporal now logs an error message if a workflow execution receives a signal that it is not able to handle.

Please see the following Temporal Documentation:

You can find more information on Temporal's official documentation site.