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().

Also fix CI: bump actions/checkout (v2 -> v7) and actions/setup-go (v5 -> v7), run checkout before setup-go with cache-dependency-path so module caching works, drop the dead glide actions/cache step (no glide.lock/vendor in the repo), and exclude stdlib internal/synctest from check_no_test_deps (Go 1.25+ pulls it into prod deps and its name trips the test grep, as internal/testlog already does).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: justinhwang

.github/workflows/tests.yaml

@@ -28,23 +28,18 @@ jobs:
           LINT: "yes"
 
     steps:
-    - name: Setup Go
-      uses: actions/setup-go@v5
-      with:
-        go-version: ${{ matrix.go }}
-
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v7
       with:
         path: ${{ env.GOPATH }}/src/github.com/${{ github.repository }}
 
-    - name: Load cache
-      uses: actions/cache@v1
+    - name: Setup Go
+      uses: actions/setup-go@v7
       with:
-        path: ~/.glide/cache
-        key: ${{ runner.os }}-go-${{ hashFiles('**/glide.lock') }}
-        restore-keys: |
-          ${{ runner.os }}-go-
+        go-version: ${{ matrix.go }}
+        # Code is checked out into $GOPATH/src (above), not the workspace root,
+        # so point setup-go's module cache at the real go.sum location.
+        cache-dependency-path: '**/go.sum'
 
     - name: Install CI
       run: make install_ci

CHANGELOG.md

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+## [Unreleased]
+### Added
+
+ * `SystemError` now implements `Is`, so `errors.Is(err, context.DeadlineExceeded)` and `errors.Is(err, context.Canceled)` match tchannel timeout and cancellation errors respectively. (#934)
+
 ## [1.34.6] - 2025-01-07
 ### Fixed
 

Makefile

@@ -85,7 +85,7 @@ test_relay_frame_leaks:
 	PATH=$(BIN):$$PATH go test -parallel=4 $(TEST_ARG) relay_test.go
 
 check_no_test_deps:
-	! go list -json $(PROD_PKGS) | jq -r '.Deps | select ((. | length) > 0) | .[]' | grep -e test -e mock | grep -v '^internal/testlog'
+	! go list -json $(PROD_PKGS) | jq -r '.Deps | select ((. | length) > 0) | .[]' | grep -e test -e mock | grep -vE '^internal/(testlog|synctest)'
 
 benchmark: clean setup $(BIN)/thrift
 	echo Running benchmarks:

errors.go

@@ -191,6 +191,22 @@ func (se SystemError) Message() string {
 	return se.msg
 }
 
+// Is lets errors.Is match a SystemError against the context sentinel its wire
+// code represents: ErrCodeTimeout matches context.DeadlineExceeded and
+// ErrCodeCancelled matches context.Canceled. Matching is keyed on the code, not
+// the message, so timeouts and cancellations from non-Go peers match too. This
+// also matches remote and relay timeouts, so callers that must tell a local
+// context expiry from a downstream one should still check ctx.Err().
+func (se SystemError) Is(target error) bool {
+	switch se.code {
+	case ErrCodeTimeout:
+		return target == context.DeadlineExceeded
+	case ErrCodeCancelled:
+		return target == context.Canceled
+	}
+	return false
+}
+
 // GetContextError converts the context error to a tchannel error.
 func GetContextError(err error) error {
 	if err == context.DeadlineExceeded {

errors_test.go

@@ -21,6 +21,9 @@
 package tchannel
 
 import (
+	"context"
+	"errors"
+	"fmt"
 	"io"
 	"regexp"
 	"testing"
@@ -73,3 +76,63 @@ func TestRelayMetricsKey(t *testing.T) {
 		assert.Equal(t, "relay-"+code.MetricsKey(), code.relayMetricsKey(), "Unexpected relay metrics key for %v", code)
 	}
 }
+
+func TestSystemErrorIs(t *testing.T) {
+	// These targets come from the standard library's context package on purpose:
+	// callers use errors.Is(err, context.DeadlineExceeded) with the stdlib
+	// sentinels, and this test proves a SystemError matches them.
+	tests := []struct {
+		name   string
+		err    error
+		target error
+		want   bool
+	}{
+		{"timeout sentinel matches DeadlineExceeded", ErrTimeout, context.DeadlineExceeded, true},
+		{"cancelled sentinel matches Canceled", ErrRequestCancelled, context.Canceled, true},
+		{"timeout does not match Canceled", ErrTimeout, context.Canceled, false},
+		{"cancelled does not match DeadlineExceeded", ErrRequestCancelled, context.DeadlineExceeded, false},
+
+		// Matching is keyed on the wire error code, not the message, so timeouts
+		// and cancellations rebuilt from the wire (including from non-Go peers
+		// that send a different message) are still recognized.
+		{"wire timeout with custom message matches DeadlineExceeded", NewSystemError(ErrCodeTimeout, "connection timed out"), context.DeadlineExceeded, true},
+		{"wire cancel with custom message matches Canceled", NewSystemError(ErrCodeCancelled, "peer cancelled"), context.Canceled, true},
+
+		// Other codes never match the context sentinels.
+		{"busy does not match DeadlineExceeded", ErrServerBusy, context.DeadlineExceeded, false},
+		{"busy does not match Canceled", ErrServerBusy, context.Canceled, false},
+		{"bad request does not match DeadlineExceeded", ErrTimeoutRequired, context.DeadlineExceeded, false},
+		{"timeout does not match an unrelated error", ErrTimeout, io.EOF, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, errors.Is(tt.err, tt.target))
+		})
+	}
+}
+
+func TestSystemErrorIsThroughWrap(t *testing.T) {
+	// errors.Is must find the context sentinel when a SystemError is wrapped
+	// further up the chain with %w.
+	err := fmt.Errorf("call to service failed: %w", ErrTimeout)
+	assert.True(t, errors.Is(err, context.DeadlineExceeded),
+		"errors.Is should see context.DeadlineExceeded through a wrapped timeout")
+
+	err = fmt.Errorf("call to service failed: %w", NewSystemError(ErrCodeCancelled, "peer cancelled"))
+	assert.True(t, errors.Is(err, context.Canceled),
+		"errors.Is should see context.Canceled through a wrapped cancellation")
+}
+
+func TestSystemErrorIdentityUnchanged(t *testing.T) {
+	// Is() is purely additive: it changes no SystemError values, so existing
+	// equality-based comparisons and code extraction keep working. A timeout
+	// rebuilt from the wire still equals the ErrTimeout sentinel by value, and
+	// the sentinels still report their codes.
+	assert.Equal(t, ErrTimeout, NewSystemError(ErrCodeTimeout, "timeout"),
+		"a rebuilt wire timeout must still equal the ErrTimeout sentinel by value")
+	assert.Equal(t, ErrRequestCancelled, NewSystemError(ErrCodeCancelled, "request cancelled"),
+		"a rebuilt wire cancellation must still equal the ErrRequestCancelled sentinel by value")
+	assert.Equal(t, ErrCodeTimeout, GetSystemErrorCode(ErrTimeout))
+	assert.Equal(t, ErrCodeCancelled, GetSystemErrorCode(ErrRequestCancelled))
+}