Description
I was recently investigating an issue that manifested as Git operations failing to complete while API access continued to function when accessing a popular Git provider. The issue was caused by SSO enforcement on the credentials being used: the provider was configured to require an SSO check every 24h for Git operations.
Unfortunately, the only messaging I was getting out of this library was "authorization failed" (coming from https://github.com/go-git/go-git/blob/master/plumbing/transport/common.go#L35).
Digging into this a bit, I found the transport/http
error handling, which discards the response body when the HTTP code maps to a known error type. This hides the message from the Git provider which is akin to "account configured for SSO" or some such.
I think a better way to handle these errors would be to wrap the common error kind using fmt.Errorf("%w: %s", err, reason)
. This is compatible with the current error output if folks are using errors.Is(err, target) bool
to check error equality (which folks should be doing).
With regards to being able to pass the reason string back to the caller: I don't expect that any Git provider would generate output for these errors that would be problematic to pass back to a consumer of the library.
eg:
// instead of this
switch r.StatusCode {
case http.StatusUnauthorized:
return transport.ErrAuthenticationRequired
case http.StatusForbidden:
return transport.ErrAuthorizationFailed
case http.StatusNotFound:
return transport.ErrRepositoryNotFound
}
// do this
switch r.StatusCode {
case http.StatusUnauthorized:
return fmt.Errorf("%w: %s", transport.ErrAuthenticationRequired, reason)
case http.StatusForbidden:
return fmt.Errorf("%w: %s", transport.ErrAuthorizationFailed, reason)
case http.StatusNotFound:
return fmt.Errorf("%w: %s", transport.ErrRepositoryNotFound, reason)
}