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

Use the same timer duration validation with server #1121

Closed demirkayaender closed 3 years ago

demirkayaender commented 3 years ago

What changed? Server performs timer duration validation against duration in seconds. However, client performs the same checks against duration type. Although, the client side checks look safe enough, we had a recent incident where server received invalid timer parameters. Although it's unlikely, this validation discrepancy probably happened due to the float to int conversion discrepancies. Therefore, we are applying the same type of validation in client to eliminate that possibility.

Why?

How did you test it?

Potential risks

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 3ba6deea-18e8-4f67-bcbb-9f4ff8d5c42d


Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/internal_event_handlers.go 7 8 87.5%
<!-- Total: 7 8 87.5% -->
Files with Coverage Reduction New Missed Lines %
internal/common/convert.go 3 84.62%
internal/internal_task_pollers.go 8 80.96%
<!-- Total: 11 -->
Totals Coverage Status
Change from base Build 6e7ee6fa-6a5e-4c8b-b873-e06b32984d79: 0.1%
Covered Lines: 12093
Relevant Lines: 16980

💛 - Coveralls
demirkayaender commented 3 years ago

@mkolodezny Good catch! I checked java client and it seems to have it already: https://github.com/uber/cadence-java-client/blob/master/src/main/java/com/uber/cadence/internal/sync/SyncDecisionContext.java#L494-L515