Skip to content

Commit a55fc1d

Browse files
justinhwangclaude
andcommitted
Add SystemError.Is for errors.Is context matching (#934)
SystemError now implements Is so errors.Is(err, context.DeadlineExceeded) and errors.Is(err, context.Canceled) match tchannel timeout and cancellation errors respectively. Matching is keyed on the wire error code, so timeouts and cancellations from non-Go peers match too. This also matches remote and relay timeouts, so callers that must distinguish a local context expiry from a downstream one should still check ctx.Err(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8f6af1a commit a55fc1d

3 files changed

Lines changed: 84 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
Changelog
22
=========
33

4+
## [Unreleased]
5+
### Added
6+
7+
* `SystemError` now implements `Is`, so `errors.Is(err, context.DeadlineExceeded)` and `errors.Is(err, context.Canceled)` match tchannel timeout and cancellation errors respectively. (#934)
8+
49
## [1.34.6] - 2025-01-07
510
### Fixed
611

errors.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,22 @@ func (se SystemError) Message() string {
191191
return se.msg
192192
}
193193

194+
// Is lets errors.Is match a SystemError against the context sentinel its wire
195+
// code represents: ErrCodeTimeout matches context.DeadlineExceeded and
196+
// ErrCodeCancelled matches context.Canceled. Matching is keyed on the code, not
197+
// the message, so timeouts and cancellations from non-Go peers match too. This
198+
// also matches remote and relay timeouts, so callers that must tell a local
199+
// context expiry from a downstream one should still check ctx.Err().
200+
func (se SystemError) Is(target error) bool {
201+
switch se.code {
202+
case ErrCodeTimeout:
203+
return target == context.DeadlineExceeded
204+
case ErrCodeCancelled:
205+
return target == context.Canceled
206+
}
207+
return false
208+
}
209+
194210
// GetContextError converts the context error to a tchannel error.
195211
func GetContextError(err error) error {
196212
if err == context.DeadlineExceeded {

errors_test.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
package tchannel
2222

2323
import (
24+
"context"
25+
"errors"
26+
"fmt"
2427
"io"
2528
"regexp"
2629
"testing"
@@ -73,3 +76,63 @@ func TestRelayMetricsKey(t *testing.T) {
7376
assert.Equal(t, "relay-"+code.MetricsKey(), code.relayMetricsKey(), "Unexpected relay metrics key for %v", code)
7477
}
7578
}
79+
80+
func TestSystemErrorIs(t *testing.T) {
81+
// These targets come from the standard library's context package on purpose:
82+
// callers use errors.Is(err, context.DeadlineExceeded) with the stdlib
83+
// sentinels, and this test proves a SystemError matches them.
84+
tests := []struct {
85+
name string
86+
err error
87+
target error
88+
want bool
89+
}{
90+
{"timeout sentinel matches DeadlineExceeded", ErrTimeout, context.DeadlineExceeded, true},
91+
{"cancelled sentinel matches Canceled", ErrRequestCancelled, context.Canceled, true},
92+
{"timeout does not match Canceled", ErrTimeout, context.Canceled, false},
93+
{"cancelled does not match DeadlineExceeded", ErrRequestCancelled, context.DeadlineExceeded, false},
94+
95+
// Matching is keyed on the wire error code, not the message, so timeouts
96+
// and cancellations rebuilt from the wire (including from non-Go peers
97+
// that send a different message) are still recognized.
98+
{"wire timeout with custom message matches DeadlineExceeded", NewSystemError(ErrCodeTimeout, "connection timed out"), context.DeadlineExceeded, true},
99+
{"wire cancel with custom message matches Canceled", NewSystemError(ErrCodeCancelled, "peer cancelled"), context.Canceled, true},
100+
101+
// Other codes never match the context sentinels.
102+
{"busy does not match DeadlineExceeded", ErrServerBusy, context.DeadlineExceeded, false},
103+
{"busy does not match Canceled", ErrServerBusy, context.Canceled, false},
104+
{"bad request does not match DeadlineExceeded", ErrTimeoutRequired, context.DeadlineExceeded, false},
105+
{"timeout does not match an unrelated error", ErrTimeout, io.EOF, false},
106+
}
107+
108+
for _, tt := range tests {
109+
t.Run(tt.name, func(t *testing.T) {
110+
assert.Equal(t, tt.want, errors.Is(tt.err, tt.target))
111+
})
112+
}
113+
}
114+
115+
func TestSystemErrorIsThroughWrap(t *testing.T) {
116+
// errors.Is must find the context sentinel when a SystemError is wrapped
117+
// further up the chain with %w.
118+
err := fmt.Errorf("call to service failed: %w", ErrTimeout)
119+
assert.True(t, errors.Is(err, context.DeadlineExceeded),
120+
"errors.Is should see context.DeadlineExceeded through a wrapped timeout")
121+
122+
err = fmt.Errorf("call to service failed: %w", NewSystemError(ErrCodeCancelled, "peer cancelled"))
123+
assert.True(t, errors.Is(err, context.Canceled),
124+
"errors.Is should see context.Canceled through a wrapped cancellation")
125+
}
126+
127+
func TestSystemErrorIdentityUnchanged(t *testing.T) {
128+
// Is() is purely additive: it changes no SystemError values, so existing
129+
// equality-based comparisons and code extraction keep working. A timeout
130+
// rebuilt from the wire still equals the ErrTimeout sentinel by value, and
131+
// the sentinels still report their codes.
132+
assert.Equal(t, ErrTimeout, NewSystemError(ErrCodeTimeout, "timeout"),
133+
"a rebuilt wire timeout must still equal the ErrTimeout sentinel by value")
134+
assert.Equal(t, ErrRequestCancelled, NewSystemError(ErrCodeCancelled, "request cancelled"),
135+
"a rebuilt wire cancellation must still equal the ErrRequestCancelled sentinel by value")
136+
assert.Equal(t, ErrCodeTimeout, GetSystemErrorCode(ErrTimeout))
137+
assert.Equal(t, ErrCodeCancelled, GetSystemErrorCode(ErrRequestCancelled))
138+
}

0 commit comments

Comments
 (0)