Thanks for this.
I don't think this is meant for Prometheus itself, which means this will always result in unnecessary traversals of the errorExec chains.
For library use, we need to consider a different approach that doesn't penalize Prometheus.

Thank you @machine424 for taking a look.
To avoid the traversals, would you be more willing to have a struct that contains error code and error chain (and make it a standard way of returning error from downstream), such that Prometheus could just read the error code directly?

Added new struct

type errorWithStatusCode struct {
    statusCode int
    err        error
}

and downstream components (prometheus dependencies) can do the following to specify HTTP error code on engine execution error:

if errWithCode, ok := grpcutil.ErrorWithHTTPStatusCode(code, err); ok {
    return errWithCode
}
return err

Thanks, but I still believe that util/grpcutil doesn't belong in the Prometheus codebase.
Instead, we could allow customization of the apiError -> status code mapping, assuming other maintainers agree to open that door.

I hadn't shared my opinion earlier, but as an end user I’d be confused e.g. if an API occasionally returned a 429 status, because the API itself is rate limited by an underlying service. Could you share more about your use case?

I'm a Cortex user, and I'd like to implement different throttling mechanisms on queryables where I want to return error code other than 422.

For example, if the data layer is hammered, I want to throttle user with 429 instead of 422, encouraging user to retry the queries at a slower pace, rather than implying the individual queries are beyond certain limits and can never succeed.

I don't believe full customization of all different apiErr.typ is necessary, but more looking for a way to override error code for errorExec type.

I believe another option is to add a new field in the API struct that contains an error code override function, but only applicable to errorExec type. Please let me know what you would prefer!

Thanks for sharing the details.
It's about making tradeoffs with such changes, we want the code to be customizable with less overhead on your end, but we also want to keep the code on our end simple and readable.
we could try replacing the switch with a lookup from map[errorType]func(errorType)int for which we can allow dependents to override entries, but we should check that the overhead is acceptable, if not we'd need to go for a default switch that you can override as a whole...

• Created ErrorTypeToStatusCode which can be passed to v1.NewAPI to override the status code based on error type and the error itself. For example,

errorTypeToStatusCode := ErrorTypeToStatusCode{
    ErrorExec: func(err error) int {
        if strings.Contains(err.Error(), "too many requests") {
            return http.StatusTooManyRequests
        }
        return http.StatusUnprocessableEntity
    }
}
api := v1.NewAPI(..., errorTypeToStatusCode)

• Exposed all errorType
• Added unit tests
• Added changelog

@machine424 Updates have been made and conflicts have been resolved. Could you take another look?

cc @bboreham as a "prometheus as a library" user, FYI

If you're going to change this, why go via strings at all? Isn't it logically more like an enum?

@bboreham Could you elaborate? The error type enums haven't been changed, and I just exposed them so that code handler could be customized.

Here's an examples uage:

errorTypeToStatusCode := ErrorTypeToStatusCode{
    ErrorExec: func(err error) int {
        if strings.Contains(err.Error(), "too many requests") {
            return http.StatusTooManyRequests
        }
        return http.StatusUnprocessableEntity
    }
}
api := v1.NewAPI(..., errorTypeToStatusCode)

where ErrorExec is an enum exported from Prometheus. New tests have been added inTestResponseError that tests the override.

I mean ErrorType would become an int type so we're not hashing string values every time we use it.

Oh I see, I believe we still need the string consts since they're used for creating apiError:

type apiError struct {
	typ errorType
	err error
}

func (e *apiError) Error() string {
	return fmt.Sprintf("%s: %s", e.typ, e.err)
}

To avoid string hashing, I can create another set of enum (const with int type), and add that to the apiError struct like this?

// existing string consts
type errorTypeStr string
const (
	errorTimeoutStr       errorTypeStr = "timeout"
	errorCanceledStr      errorTypeStr = "canceled"
	...
)

// new enum exported
type errorType int
const (
	ErrorTimeout       errorType = iota
	ErrorCanceled
	...
)

// update apiError struct to include both error string and type
type apiError struct {
	typ errorType
	typStr errorTypeStr
	err error
}

...

if handler, ok := api.errorTypeToStatusCode[apiErr.typ]; ok {
	code = handler(apiErr.err)
} else {
	switch apiErr.typStr {
	case errorTimeoutStr:
        ...
	}
}

Please let me know if this alternative looks good to you

Updated errorType to a struct that contains both int and string. The int values for all error types are exported, and ErrorTypeToStatusCode uses the exported error type int as the map key.

@machine424 @bboreham Could you take another look?

@machine424 @bboreham could I get final reviews?