Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix API Error Wrappers #501

Open
winder opened this issue Apr 6, 2023 · 1 comment
Open

Fix API Error Wrappers #501

winder opened this issue Apr 6, 2023 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers Team Lamprey

Comments

@winder
Copy link
Contributor

winder commented Apr 6, 2023

Subject of the issue

The way API errors are wrapped does not work. Having type aliases to an interface means that at runtime all of the errors are indistinguishable from one another.

Here is the current code:

type BadRequest error
type InvalidToken error
type NotFound error
type InternalError error

// extractError checks if the response signifies an error.
// If so, it returns the error.
// Otherwise, it returns nil.
func extractError(code int, errorBuf []byte) error {
	if code >= 200 && code < 300 {
		return nil
	}

	wrappedError := fmt.Errorf("HTTP %v: %s", code, errorBuf)
	switch code {
	case 400:
		return BadRequest(wrappedError)
	case 401:
		return InvalidToken(wrappedError)
	case 404:
		return NotFound(wrappedError)
	case 500:
		return InternalError(wrappedError)
	default:
		return wrappedError
	}
}

Solution

Create a real error and use that for the specific types. They should also include the error code, for example:

// You can edit this code!
// Click here and start typing.
package main

import "fmt"

type HTTPError struct {
	Msg  string
	Code uint
}

func (e HTTPError) Error() string {
	return fmt.Sprintf("Client error (%d): %s", e.Code, e.Msg)
}

type NotFound HTTPError

func (e NotFound) Error() string {
	return fmt.Sprintf("NotFound error (%d): %s", e.Code, e.Msg)
}

func main() {
	genericError := HTTPError{
		Msg:  "Method Not Found",
		Code: 404,
	}

	fmt.Printf("Error type: %T, Error message: %v\n", genericError, genericError)

	var notFound NotFound = NotFound{
		Msg:  "Method Not Found",
		Code: 404,
	}
	fmt.Printf("Error type: %T, Error message: %v\n", notFound, notFound)
}

Create types so that the extractError can be updated as follows:

// extractError checks if the response signifies an error.
// If so, it returns the error.
// Otherwise, it returns nil.
func extractError(code int, errorBuf []byte) error {
	if code >= 200 && code < 300 {
		return nil
	}

	switch code {
	case 401:
		return InvalidToken{Msg: errorBuf}
	case 404:
		return NotFound{Msg: errorBuf}
	case 500:
		return InternalError(wrappedError)
	default:
		if code < 400 {
			return ClientError{Code: code, Msg: errorBuf}
		} else if code < 500 {
			return ServerError{Code: code, Msg: errorBuf}
		}
		return UnknownError{Code: code, Msg: errorBuf}
	}
}
@bbroder-algo
Copy link

nice catch!

@algoanne algoanne added the tech debt Things that need re-work for simplification / sanitization to reduce implementation overhead label Apr 18, 2023
@winder winder added bug Something isn't working and removed tech debt Things that need re-work for simplification / sanitization to reduce implementation overhead labels Apr 20, 2023
@algoanne algoanne added the good first issue Good for newcomers label May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Team Lamprey
Projects
None yet
Development

No branches or pull requests

3 participants