fix(catalog/rest): keep status override when error body is malformed

[ryanworl]

What

handleNon200 returned early when the Iceberg error-envelope JSON failed to decode, before applying the per-status override map. A 5xx commit response whose body a proxy or load balancer replaced with an HTML error page (or truncated JSON) therefore surfaced as a bare ErrRESTError decode failure instead of ErrCommitStateUnknown.

Callers classify ErrCommitStateUnknown as an ambiguous commit — the write may or may not have landed — and handle it accordingly. Losing that sentinel turns a transient catalog/proxy fault into a fatal error after one attempt; transport garbling should not strip commit-state classification.

On a non-EOF decode failure, handleNon200 now looks up the caller-supplied override[status] and wraps that sentinel (else ErrRESTError), still wrapping the errorResponse so the HTTP status and Retry-After survive for pollers like WaitForPlan. Mapped 5xx commits stay ErrCommitStateUnknown, mapped 409s stay ErrCommitFailed, unmapped statuses still classify as ErrRESTError, and the decode failure remains in the error message.

Tests

• Table-driven TestHandleNon200_StatusOverrideAppliesOnMalformedBody: well-formed vs malformed 500 both map to ErrCommitStateUnknown; well-formed vs malformed 409 both map to ErrCommitFailed; unmapped malformed 404 stays ErrRESTError.
• TestCommitTableErrorBodyKeepsCommitStateUnknown pins the same well-formed/malformed pairs end-to-end through CommitTable for 500/502/503/504.
• go test ./catalog/... and golangci-lint run are clean.

Made with Cursor

[zeroshade]

The fix is correct and minimal: on a non-EOF decode failure handleNon200 now consults the caller's override[status] map before defaulting to ErrRESTError (catalog/rest/rest.go:604-606), which mirrors the well-formed-body path below and restores ErrCommitStateUnknown / ErrCommitFailed classification when a proxy garbles the error body. Since ErrCommitStateUnknown itself wraps ErrRESTError, existing errors.Is(err, ErrRESTError) checks are unaffected, and the errorResponse wrapper keeps the status code and Retry-After for WaitForPlan. Skipping the typeOverride refinement in this path is right, too — there's no decoded error.type to refine on. Test coverage is thorough at both the unit and end-to-end CommitTable levels. I verified go test ./catalog/rest/ passes on the PR head.

One non-blocking observation: the malformed-body path still bypasses the default status-based switch for statuses the caller didn't map — e.g. a malformed 503 on a non-commit call classifies as ErrRESTError rather than ErrServiceUnavailable, while a well-formed 503 falls through to the switch. That's pre-existing behavior and out of scope here, but if we ever want full parity between the two paths it might be cleaner to set e.wrapping via the shared classification logic instead of duplicating the override lookup. Fine to leave as-is.