Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions examples/spec/integration/activity_cancellation_spec.rb
Original file line number Diff line number Diff line change
@@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always failing for me, locally

  1) Activity cancellation cancels a running activity
     Failure/Error: expect(result).to be_a(LongRunningActivity::Canceled)
       expected #<Temporal::ActivityCanceled: ACTIVITY_ID_NOT_STARTED> to be a kind of LongRunningActivity::Canceled
     # ./spec/integration/activity_cancellation_spec.rb:19:in `block (2 levels) in <top (required)>'

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
2 changes: 1 addition & 1 deletion examples/workflows/long_workflow.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ def execute(cycles = 10, interval = 1)
future.cancel
end

future.wait
future.get
end
end
3 changes: 3 additions & 0 deletions lib/temporal/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since cancels can also happen at a heartbeat, should we unify this case with that case? It's not ideal that people would have to deal with both depending on the timing of when the cancellation comes in.
If that's not possible, should name it as ActivityCanceledBeforeStart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing preventing other to re-use the Temporal::ActivityCancelled error for heartbeat cancellations. It's just that a heartbeat cancellation is no different from any other failure raised by an activity. I think a more interesting question here is — should we provide a #cancel! API for the activity context that would raise that error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot that in this SDK, heartbeat cancel doesn't throw, it returns a boolean.
In our wrapper SDK (and in the java SDK), it will throw. We think this is the best way to make sure developers don't drop cancellations.

class ActivityCanceled < ActivityException; end

class ActivityNotRegistered < ClientError; end
class WorkflowNotRegistered < ClientError; end

Expand Down
2 changes: 1 addition & 1 deletion lib/temporal/workflow/history/event_target.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/temporal/workflow/state_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions spec/fabricators/grpc/history_event_fabricator.rb
Original file line number Diff line number Diff line change
@@ -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 }
Expand Down Expand Up @@ -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|
Expand Down
16 changes: 16 additions & 0 deletions spec/unit/lib/temporal/workflow/history/event_target_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 activity' 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_activity_request' do
expect(subject.type).to eq(described_class::CANCEL_ACTIVITY_REQUEST_TYPE)
end
end
end
end