feat(client): Use runId only in handles created with getHandle

[bergundy]

In addition:

• Adds safety to terminate and cancel so handles created with start can't accidentally affect workflows that are not part of the same execution chain
• Adds optional firstExecutionRunId param to getHandle for added safety
• Closes [Bug] WorkflowExecutionFailedEventAttributes.retry_state not respected #464
• Closes Improve error message: Workflow executionsRow not found #377
• Closes [Feature Request] Better context for grpc errors in client #365

[swyxio]

when should we tell people to use this in docs?

[bergundy]

When they want to make sure that they don't terminate / cancel a workflow if it's not part of a specific execution chain.

[cretz]

Re temporalio/sdk-python#6 (comment), I think we might need to revisit this.

For handles returned by start and signal-with-start, we are allowing query, signal, cancel (for signal-with-start), and terminate (for signal-with-start) to occur on a completely different workflow. We should default to using the run ID for all of those I think.

// Server 1

const handle = await client.start(nuclearWorkflow,
  { args: ['do not detonate'], taskQueue: 'task-queue', workflowId: 'nuclear-workflow' });

// Meanwhile on server 2

const handle = await client.start(nuclearWorkflow,
  { args: ['detonate'], taskQueue: 'task-queue', workflowId: 'nuclear-workflow' });

// Back on server 1

if (await handle.query('should detonate')) {
  launch();
}

You just ended the world :-)

[lorensr]

For startHandle.terminate(), we guard against terminating an out-of-chain workflow by including firstExecutionRunId, which the server checks before terminating?

Ideally (may require server change?) we could include firstExecutionRunId with query and signal? And when signal-with-starting, we either get back the firstExecutionRunId from the server, or we include applyIfChainIncludesRunId: runId with query/signal/cancel/terminate?

[cretz]

For startHandle.terminate(), we guard against terminating an out-of-chain workflow by including firstExecutionRunId, which the server checks before terminating?

Not good enough. Firstly that's only for terminate and cancel, not for query and signal. Secondly that doesn't apply to

Ideally (may require server change?) [...]

Yes, there are some changes on the server that can improve this. But as of now in master, the handle returned from start/signal-with-start query of this repo, signal, cancel (signal-with-start only), and terminate (signal-with-start only) can perform an action on a workflow started on another client that is unrelated except by its workflow ID. That is bad I think.

[bergundy]

I'm not convinced you're right @cretz.
It really depends on the use case.
AFAIK, in a lot of cases users don't reuse the same workflow ID for different execution chains.

[bergundy]

I think this change is a step in the right direction and we'll make it safer with server changes.

[cretz]

After discussion, while I disagree, the team has decided that for now it's ok to silently and accidentally signal, query, cancel, or terminate incorrect workflows until the server change is in place.