Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

plumbing/transport/http: error handling swallows useful debugging information #1097

Copy link
Copy link
Closed
@ggambetti

Description

@ggambetti
Issue body actions

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)
	}

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.