search

uber-go / cadence-client

Framework for authoring workflows and activities running on top of the Cadence orchestration engine.
https://cadenceworkflow.io
MIT License
345 stars 131 forks source link

Fix NPE when canceling workflow without run id #1150

Closed vytautas-karpavicius closed 2 years ago

vytautas-karpavicius commented 2 years ago

What changed? Fixed NPE in proto mapper.

Why? When canceling current workflow (without run id), this field can be nil.

How did you test it? Added new integration test to reproduce, fix and verify.

Potential risks

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1755f1ca-f29b-4a53-b497-ed6f747ebf00

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/compatibility/proto/types.go 4 5 80.0%
internal/compatibility/proto/request.go 0 4 0.0%
<!-- Total: 5 10 50.0% -->
Totals Coverage Status
Change from base Build 4f32e7fd-8f48-4ad7-9dd0-cf6c681c5e48: 0.5%
Covered Lines: 12362
Relevant Lines: 19363
💛 - Coveralls
vytautas-karpavicius commented 2 years ago

Do we have the same issue for signaling external workflow? I think runID can also be empty there.

RunId can be empty there as well. But it is slightly different. Because workflowId/runId are both with WorkflowExecution, so they map 1:1 for proto/thrift. For cancelation WorkflowID, RunID fields were directly on request type, thus a different mapper being used.