uber-go / cadence-client

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

Moving retryable-err checks to errors.As, moving some to not-retryable #1167

Closed Groxx closed 2 years ago

Groxx commented 2 years ago

Part 1 of 2 for solving retry storms, particularly around incorrectly-categorized errors (e.g. limit exceeded) and service-busy.

This PR moves us to errors.As to support wrapped errors in the future, and re-categorizes some incorrectly-retried errors. This is both useful on its own, and helps make #1174 a smaller and clearer change.

Service-busy behavior is actually changed in #1174, this PR intentionally maintains its current (flawed) behavior.

Commits are ordered for ease of reading / verifying, and the added tests pass at each stage (but I did not run the full suite each time):

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 0181880b-5443-4634-9400-8e5f001cb004


Files with Coverage Reduction New Missed Lines %
internal/compatibility/thrift/history.go 11 57.65%
<!-- Total: 11 -->
Totals Coverage Status
Change from base Build 01816e39-350f-43ae-a79e-40eaad99261d: 0.1%
Covered Lines: 12433
Relevant Lines: 19472

💛 - Coveralls