Bug #16665
closedKeepproxy reports error 413 (entity too large) even if the original error was something else
Description
This makes debugging really annoying.
Updated by Lucas Di Pentima over 3 years ago
Maybe related to this: go-staticcheck
(via vscode) is showing me a warning that says:
unreachable case clause: git.arvados.org/arvados.git/sdk/go/keepclient.OversizeBlockError will always match before git.arvados.org/arvados.git/sdk/go/keepclient.InsufficientReplicasError (SA4020)
...for the case keepclient.InsufficientReplicasError:
in keepproxy.go
's Put() function. Both types are aliases of error
so it seems the compiler won't take them as different.
Updated by Peter Amstutz over 3 years ago
type InsufficientReplicasError error type OversizeBlockError error switch err.(type) { case nil: status = http.StatusOK _, err = io.WriteString(resp, locatorOut) case keepclient.OversizeBlockError: // Too much data status = http.StatusRequestEntityTooLarge case keepclient.InsufficientReplicasError: }
I think this is an inconsistency in Go. If you declare a type that is the same as another type, at compile time they will act like different types, but at runtime they will be the same type.
Does this work?
type InsufficientReplicasError struct { error }
Updated by Peter Amstutz over 3 years ago
- Target version changed from Arvados Future Sprints to 2021-07-21 sprint
Updated by Peter Amstutz over 3 years ago
- Target version changed from 2021-07-21 sprint to 2021-08-04 sprint
Updated by Lucas Di Pentima over 3 years ago
- Status changed from New to In Progress
Updated by Lucas Di Pentima over 3 years ago
Updates at e10c23d41 - branch 16665-keepproxy-spurious-413-status
Test run: developer-run-tests: #2606
- Following Peter's suggestion, updates
InsufficientReplicasError
&OversizeBlockError
error types by wrappingerror
in a struct, so that the former isn't shadowed by the latter. - Updates tests.
Updated by Tom Clegg over 3 years ago
LGTM, thanks.
BTW, about the language issue: error is an interface type, therefore type FooError error
defines an interface type, therefore anything that implements the error interface { Error() string } has to match FooError in a type switch. After this fix, type FooError struct { error }
is a concrete type, so FooError in a type switch is unambiguous. I think it would have been more normal to use var ErrFoo = errors.New(...)
and if err == ErrFoo { ... }
for these... oh well.
Updated by Lucas Di Pentima over 3 years ago
- % Done changed from 0 to 100
- Status changed from In Progress to Resolved
Applied in changeset arvados|68b9c7d30c7f47f8a9f9cff8a327fa9a3812d4da.