feat(errs): classify version mismatches as retryable

Author: albertywu

platform/errs/README.md

@@ -12,7 +12,7 @@ Errors are classified along two axes:
 | **Infra**     | *(any unclassified error)* | `NewRetryableError`           |
 | **Infra dep** | `NewDependencyError`    | `NewRetryableDependencyError`    |
 
-**Non-retryable by default.** A plain `fmt.Errorf(...)` is treated as a non-retryable infra error. Retryability must be explicitly opted into by wrapping with `NewRetryableError`. This prevents accidental infinite retry loops from unclassified errors.
+**Non-retryable by default.** A plain `fmt.Errorf(...)` is treated as a non-retryable infra error. Retryability must be explicitly recognized by a registered classifier or framework wrapper. This prevents accidental infinite retry loops from unclassified errors.
 
 **Only infra errors can be retryable.** User errors are never retryable — if a user action caused the failure, retrying the same operation will produce the same result. If an error is retryable, it is by definition an infrastructure issue.
 
@@ -58,7 +58,7 @@ Two implementations ship in this package:
 
 ## Adding a Backend-Specific Classifier
 
-Backend classifiers live alongside the extension they classify, under `platform/errs/<backend>/`. The canonical examples are `platform/errs/mysql` (MySQL driver errors) and `platform/errs/generic` (transport-agnostic concerns such as `context.Canceled`).
+Backend classifiers live alongside the extension they classify, under `platform/errs/<backend>/`. The canonical examples are `platform/errs/mysql` (MySQL driver errors) and `platform/errs/generic` (backend-independent errors such as `context.Canceled` and `errs.ErrVersionMismatch`).
 
 A classifier:
 
@@ -137,7 +137,7 @@ In particular, **do not reach for `NewRetryableError` just because replaying the
 
 ## Extensions Return Plain Go Errors
 
-Extension interfaces (`MergeChecker`, `Storage`, `Publisher`) return standard `error` values. They may define their own domain-specific sentinel errors (e.g. `storage.ErrNotFound`, `storage.ErrVersionMismatch`) but they do **not** classify errors as user or infra — that is the controller's (and the consumer's `ErrorProcessor`'s) job.
+Extension interfaces (`MergeChecker`, `Storage`, `Publisher`) return standard `error` values. They may define domain-specific sentinel errors such as `storage.ErrNotFound` or expose shared semantic sentinels such as `errs.ErrVersionMismatch`, but they do **not** classify errors as user or infra. That is the controller's and the consumer's `ErrorProcessor`'s job.
 
 This separation keeps extensions reusable across contexts. The same `storage.ErrNotFound` might be a user error in one controller (user requested a non-existent resource) and an infra error in another (expected record is missing).
 

platform/errs/errs.go

@@ -18,6 +18,10 @@ import (
 	"errors"
 )
 
+// ErrVersionMismatch indicates that an optimistic conditional update failed
+// because the persisted version no longer matches the expected version.
+var ErrVersionMismatch = errors.New("version mismatch")
+
 // userError represents an error caused by invalid user input or actions.
 // User errors are never retryable — only infrastructure errors can be retryable.
 // Use NewUserError to wrap an underlying cause.

platform/errs/generic/generic.go

@@ -39,6 +39,10 @@ type classifier struct{}
 // must not call errors.Is / errors.As — the classifier-processor owns the
 // chain walk.
 func (classifier) Classify(err error) errs.Verdict {
+	if err == errs.ErrVersionMismatch {
+		return errs.InfraRetryable
+	}
+
 	// Cancellation signals that the caller aborted the work in flight
 	// (process shutdown, deadline on the inbound RPC, parent operation gone) —
 	// it is not a statement about the work itself being invalid. The same

platform/errs/generic/generic_test.go

@@ -24,8 +24,20 @@ import (
 	"github.com/uber/submitqueue/platform/errs"
 )
 
-func TestClassifier_ContextCanceled(t *testing.T) {
-	assert.Equal(t, errs.InfraRetryable, Classifier.Classify(context.Canceled))
+func TestClassifier_Retryable(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+	}{
+		{"context canceled", context.Canceled},
+		{"version mismatch", errs.ErrVersionMismatch},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, errs.InfraRetryable, Classifier.Classify(tt.err))
+		})
+	}
 }
 
 func TestClassifier_Unknown(t *testing.T) {
@@ -37,6 +49,7 @@ func TestClassifier_Unknown(t *testing.T) {
 		// context.Canceled; the surrounding classifier-processor walk will
 		// reach the inner node and ask Classifier again there.
 		{"wrapped context.Canceled", fmt.Errorf("op: %w", context.Canceled)},
+		{"wrapped version mismatch", fmt.Errorf("op: %w", errs.ErrVersionMismatch)},
 		{"deadline exceeded", context.DeadlineExceeded},
 		{"plain error", errors.New("anything")},
 		{"nil", nil},
@@ -65,6 +78,12 @@ func TestClassifier_AppliedViaProcessor(t *testing.T) {
 		assert.True(t, errs.IsRetryable(out))
 	})
 
+	t.Run("wrapped version mismatch becomes retryable infra", func(t *testing.T) {
+		wrapped := fmt.Errorf("update: %w", errs.ErrVersionMismatch)
+		out := p.Process(wrapped)
+		assert.True(t, errs.IsRetryable(out))
+	})
+
 	t.Run("framework wrap in chain wins", func(t *testing.T) {
 		// A controller explicitly classified the shutdown as non-retryable.
 		// The pass-1 framework-wrap check short-circuits before Classifier

stovepipe/extension/storage/BUILD.bazel

@@ -11,5 +11,8 @@ go_library(
     ],
     importpath = "github.com/uber/submitqueue/stovepipe/extension/storage",
     visibility = ["//visibility:public"],
-    deps = ["//stovepipe/entity:go_default_library"],
+    deps = [
+        "//platform/errs:go_default_library",
+        "//stovepipe/entity:go_default_library",
+    ],
 )

stovepipe/extension/storage/storage.go

@@ -19,6 +19,8 @@ package storage
 import (
 	"errors"
 	"fmt"
+
+	platformerrs "github.com/uber/submitqueue/platform/errs"
 )
 
 // ErrNotFound is returned by storage implementations when the requested record is not found in the database.
@@ -40,7 +42,7 @@ var ErrAlreadyExists = errors.New("record already exists")
 // ErrVersionMismatch is returned by storage implementations when a conditional (CAS) update finds that
 // the stored version does not match the expected version. It backs optimistic locking, letting callers
 // retry or converge instead of overwriting a concurrent change.
-var ErrVersionMismatch = errors.New("version mismatch")
+var ErrVersionMismatch = platformerrs.ErrVersionMismatch
 
 // Storage is a factory interface that aggregates all entity stores into a single injectable dependency.
 type Storage interface {

submitqueue/extension/storage/BUILD.bazel

@@ -16,5 +16,8 @@ go_library(
     ],
     importpath = "github.com/uber/submitqueue/submitqueue/extension/storage",
     visibility = ["//visibility:public"],
-    deps = ["//submitqueue/entity:go_default_library"],
+    deps = [
+        "//platform/errs:go_default_library",
+        "//submitqueue/entity:go_default_library",
+    ],
 )

submitqueue/extension/storage/storage.go

@@ -19,6 +19,8 @@ package storage
 import (
 	"errors"
 	"fmt"
+
+	platformerrs "github.com/uber/submitqueue/platform/errs"
 )
 
 // ErrNotFound is returned by storage implementations when the requested record is not found in the database.
@@ -40,7 +42,7 @@ var ErrAlreadyExists = errors.New("record already exists")
 // ErrVersionMismatch is returned by storage implementations when the expected entity version does not match the current version of the object.
 // This is used to implement an optimistic locking mechanism, allowing multiple clients to update the same entity concurrently
 // and either retry or implement idempotent operations.
-var ErrVersionMismatch = errors.New("version mismatch")
+var ErrVersionMismatch = platformerrs.ErrVersionMismatch
 
 // Storage is a factory interface that aggregates all entity stores into a single injectable dependency.
 type Storage interface {