yylego / restyoops

93%

Build Type

push

github

Committed by yyle88

Commit Message

redesign restyoops around resty's own retry engine

The package answered "is this retryable" and then stopped, leaving the caller to
write the loop, the sleeping and the counting by hand. resty already owns all
three. What resty leaves to its users is the judgement, and its out-of-box
judgement is wrong in ways that are easy to miss, each of which is now a test:

- a 500 is never repeated, so SetRetryCount looks set up yet does nothing
- resty's own AddRetryAfterErrorCondition silences retries on transport faults,
  because every condition overwrites resty's "repeat transport faults" default
- the same helper repeats a 404 four times
- Retry-After is never read, so a peer asking for 120s gets hit after 102ms
- an untrusted certificate is repeated, though trust never arrives that way
- a POST is repeated after a 500, though the order may already have been placed

Setup(client) now installs the judgement in one call and reuses resty's loop,
backoff and attempt counting rather than rebuilding them.

Fixed along the way:

- Oops implements error and unwraps to Cause, so it travels ordinary error paths
- Cause carries the real cause instead of the placeholder errors.New("HTTP")
- transport branches reordered: url.Error no longer shadows every precise
  judgement, and x509 / bad scheme / NXDOMAIN / canceled context all stop
- context.Canceled is no longer classified as a retryable network fault
- a stated zero wait stays a stated zero instead of becoming a default
- Kind is open: a custom kind no longer panics on the caller
- checks see the whole response, headers and request included, not just the body
- RetryMaxWaitTime is raised so a stated wait is not cut down to resty's 2s

Removed: the OopsIssue alias, six unused Kind predicates, internal/utils, and
the must dependency, which took zaplog / mutexmap / zap / multierr with it.
Dependencies went from ten to two.

Coverage 66.3% -> 92.3%, with the previously untested surface now covered:
content checks, ki... (continued)

Coverage Stats

337 of 363 new or added lines in 8 files covered. (92.84%)

2 existing lines in 1 file now uncovered.

348 of 376 relevant lines covered (92.55%)

61.22 hits per line