From 5948535412c751b29c9d27b4981b6b051c008d56 Mon Sep 17 00:00:00 2001 From: antstorm Date: Tue, 7 Dec 2021 12:29:34 +0000 Subject: [PATCH 1/3] Fix event target map entry for ACTIVITY_CANCELED event --- lib/temporal/workflow/history/event_target.rb | 2 +- .../grpc/history_event_fabricator.rb | 26 +++++++++++++++++++ .../workflow/history/event_target_spec.rb | 16 ++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/temporal/workflow/history/event_target.rb b/lib/temporal/workflow/history/event_target.rb index 96d5de93..8f605a01 100644 --- a/lib/temporal/workflow/history/event_target.rb +++ b/lib/temporal/workflow/history/event_target.rb @@ -19,7 +19,7 @@ class UnexpectedEventType < InternalError; end # NOTE: The order is important, first prefix match wins (will be a longer match) TARGET_TYPES = { - 'ACTIVITY_TASK_CANCEL' => CANCEL_ACTIVITY_REQUEST_TYPE, + 'ACTIVITY_TASK_CANCEL_REQUESTED' => CANCEL_ACTIVITY_REQUEST_TYPE, 'ACTIVITY_TASK' => ACTIVITY_TYPE, 'REQUEST_CANCEL_ACTIVITY_TASK' => CANCEL_ACTIVITY_REQUEST_TYPE, 'TIMER_CANCELED' => CANCEL_TIMER_REQUEST_TYPE, diff --git a/spec/fabricators/grpc/history_event_fabricator.rb b/spec/fabricators/grpc/history_event_fabricator.rb index a290ea0f..9e8538eb 100644 --- a/spec/fabricators/grpc/history_event_fabricator.rb +++ b/spec/fabricators/grpc/history_event_fabricator.rb @@ -1,5 +1,9 @@ require 'securerandom' +class TestSerializer + extend Temporal::Concerns::Payloads +end + Fabricator(:api_history_event, from: Temporal::Api::History::V1::HistoryEvent) do event_id { 1 } event_time { Time.now } @@ -122,6 +126,28 @@ end end +Fabricator(:api_activity_task_canceled_event, from: :api_history_event) do + event_type { Temporal::Api::Enums::V1::EventType::EVENT_TYPE_ACTIVITY_TASK_CANCELED } + activity_task_canceled_event_attributes do |attrs| + Temporal::Api::History::V1::ActivityTaskCanceledEventAttributes.new( + details: TestSerializer.to_details_payloads('ACTIVITY_ID_NOT_STARTED'), + scheduled_event_id: attrs[:event_id] - 2, + started_event_id: nil, + identity: 'test-worker@test-host' + ) + end +end + +Fabricator(:api_activity_task_cancel_requested_event, from: :api_history_event) do + event_type { Temporal::Api::Enums::V1::EventType::EVENT_TYPE_ACTIVITY_TASK_CANCEL_REQUESTED } + activity_task_cancel_requested_event_attributes do |attrs| + Temporal::Api::History::V1::ActivityTaskCancelRequestedEventAttributes.new( + scheduled_event_id: attrs[:event_id] - 1, + workflow_task_completed_event_id: attrs[:event_id] - 2, + ) + end +end + Fabricator(:api_timer_started_event, from: :api_history_event) do event_type { Temporal::Api::Enums::V1::EventType::EVENT_TYPE_TIMER_STARTED } timer_started_event_attributes do |attrs| diff --git a/spec/unit/lib/temporal/workflow/history/event_target_spec.rb b/spec/unit/lib/temporal/workflow/history/event_target_spec.rb index 717c4572..b7f9c385 100644 --- a/spec/unit/lib/temporal/workflow/history/event_target_spec.rb +++ b/spec/unit/lib/temporal/workflow/history/event_target_spec.rb @@ -21,5 +21,21 @@ expect(subject.type).to eq(described_class::CANCEL_TIMER_REQUEST_TYPE) end end + + context 'when event is ACTIVITY_CANCELED' do + let(:raw_event) { Fabricate(:api_activity_task_canceled_event) } + + it 'sets type to timer' do + expect(subject.type).to eq(described_class::ACTIVITY_TYPE) + end + end + + context 'when event is ACTIVITY_TASK_CANCEL_REQUESTED' do + let(:raw_event) { Fabricate(:api_activity_task_cancel_requested_event) } + + it 'sets type to cancel_timer_request' do + expect(subject.type).to eq(described_class::CANCEL_ACTIVITY_REQUEST_TYPE) + end + end end end From 6b35b495e030b270dfe0e038b19b0f38fc4eb7b7 Mon Sep 17 00:00:00 2001 From: antstorm Date: Tue, 7 Dec 2021 12:57:32 +0000 Subject: [PATCH 2/3] Fix cancellation of a non-started activity --- .../integration/activity_cancellation_spec.rb | 39 +++++++++++++++++++ examples/workflows/long_workflow.rb | 2 +- lib/temporal/errors.rb | 3 ++ lib/temporal/workflow/state_manager.rb | 2 +- 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 examples/spec/integration/activity_cancellation_spec.rb diff --git a/examples/spec/integration/activity_cancellation_spec.rb b/examples/spec/integration/activity_cancellation_spec.rb new file mode 100644 index 00000000..0d551a19 --- /dev/null +++ b/examples/spec/integration/activity_cancellation_spec.rb @@ -0,0 +1,39 @@ +require 'workflows/long_workflow' + +describe 'Activity cancellation' do + let(:workflow_id) { SecureRandom.uuid } + + it 'cancels a running activity' do + run_id = Temporal.start_workflow(LongWorkflow, options: { workflow_id: workflow_id }) + + # Signal workflow after starting, allowing it to schedule the first activity + sleep 0.5 + Temporal.signal_workflow(LongWorkflow, :CANCEL, workflow_id, run_id) + + result = Temporal.await_workflow_result( + LongWorkflow, + workflow_id: workflow_id, + run_id: run_id, + ) + + expect(result).to be_a(LongRunningActivity::Canceled) + expect(result.message).to eq('cancel activity request received') + end + + it 'cancels a non-started activity' do + # Workflow is started with a signal which will cancel an activity before it has started + run_id = Temporal.start_workflow(LongWorkflow, options: { + workflow_id: workflow_id, + signal_name: :CANCEL + }) + + result = Temporal.await_workflow_result( + LongWorkflow, + workflow_id: workflow_id, + run_id: run_id, + ) + + expect(result).to be_a(Temporal::ActivityCanceled) + expect(result.message).to eq('ACTIVITY_ID_NOT_STARTED') + end +end diff --git a/examples/workflows/long_workflow.rb b/examples/workflows/long_workflow.rb index 3682fad9..c4b4682f 100644 --- a/examples/workflows/long_workflow.rb +++ b/examples/workflows/long_workflow.rb @@ -9,6 +9,6 @@ def execute(cycles = 10, interval = 1) future.cancel end - future.wait + future.get end end diff --git a/lib/temporal/errors.rb b/lib/temporal/errors.rb index 5730224c..2e7f4ebe 100644 --- a/lib/temporal/errors.rb +++ b/lib/temporal/errors.rb @@ -19,6 +19,9 @@ class TimeoutError < ClientError; end # with the intent to propagate to a workflow class ActivityException < ClientError; end + # Represents cancellation of a non-started activity + class ActivityCanceled < ActivityException; end + class ActivityNotRegistered < ClientError; end class WorkflowNotRegistered < ClientError; end diff --git a/lib/temporal/workflow/state_manager.rb b/lib/temporal/workflow/state_manager.rb index 7d515c06..ac4c1846 100644 --- a/lib/temporal/workflow/state_manager.rb +++ b/lib/temporal/workflow/state_manager.rb @@ -162,7 +162,7 @@ def apply_event(event) when 'ACTIVITY_TASK_CANCELED' state_machine.cancel - dispatch(target, 'failed', Temporal::Workflow::Errors.generate_error(event.attributes.failure)) + dispatch(target, 'failed', Temporal::ActivityCanceled.new(from_details_payloads(event.attributes.details))) when 'TIMER_STARTED' state_machine.start From d8c68d24424c46b4b88c1e42184dc92362a3984f Mon Sep 17 00:00:00 2001 From: antstorm Date: Thu, 9 Dec 2021 18:00:54 +0000 Subject: [PATCH 3/3] fixup! Fix event target map entry for ACTIVITY_CANCELED event --- spec/unit/lib/temporal/workflow/history/event_target_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/lib/temporal/workflow/history/event_target_spec.rb b/spec/unit/lib/temporal/workflow/history/event_target_spec.rb index b7f9c385..2f2c80bd 100644 --- a/spec/unit/lib/temporal/workflow/history/event_target_spec.rb +++ b/spec/unit/lib/temporal/workflow/history/event_target_spec.rb @@ -25,7 +25,7 @@ context 'when event is ACTIVITY_CANCELED' do let(:raw_event) { Fabricate(:api_activity_task_canceled_event) } - it 'sets type to timer' do + it 'sets type to activity' do expect(subject.type).to eq(described_class::ACTIVITY_TYPE) end end @@ -33,7 +33,7 @@ context 'when event is ACTIVITY_TASK_CANCEL_REQUESTED' do let(:raw_event) { Fabricate(:api_activity_task_cancel_requested_event) } - it 'sets type to cancel_timer_request' do + it 'sets type to cancel_activity_request' do expect(subject.type).to eq(described_class::CANCEL_ACTIVITY_REQUEST_TYPE) end end