abemedia / go-don

API framework written in Golang.
https://pkg.go.dev/github.com/abemedia/go-don
MIT License
51 stars 5 forks source link

Consider rfc7807 error response handling #6

Open jarrettv opened 2 years ago

jarrettv commented 2 years ago

It appears don will fallback to a 500 error and uses a simple format { "message": "Internal Server Error" }

It would be nice if it used a standard response format with a method of hooking in validation errors like shown in the spec.

Full example below

package settings

import (
    "context"
    "github.com/Example/go-auth/auth"
    "github.com/abemedia/go-don/problems"
)

type SettingsGetReq struct {
    Session auth.Session `context:"auth-session"`
    Code    string       `path:"code"`
}

type SettingsPutReq struct {
    TargetCode string       `path:"code"`
    Session    auth.Session `context:"auth-session"`
    Settings
}

func SettingsGet(ctx context.Context, req SettingsGetReq) (*Settings, error) {
    if !req.Session.Permit("settings-read") {
        return nil, problems.NewPermError(req.Session.Username)
    }

    s, err := get(req.Code)

    if s == nil && err == nil {
        return nil, problems.NewNotFoundError()
    } else if err != nil {
        return nil, problems.NewUnexpError(err)
    }

    return s, nil
}

func SettingsPut(ctx context.Context, req SettingsPutReq) (*Settings, error) {
    if !req.Session.Permit("settings-manage") {
        return nil, problems.NewPermError(req.Session.Username)
    }

    fieldErrors := req.validate(req.TargetCode)
    if fieldErrors != nil {
        return nil, problems.NewValidError(fieldErrors)
    }

    err := put(&req.Settings)
    if err != nil {
        return nil, problems.NewUnexpError(err)
    }

    return &req.Settings, nil
}
jarrettv commented 2 years ago

If there was an easy way to take over the default error handler. Maybe something similar to panic handler?

abemedia commented 2 years ago

An error handler is a great idea. I've been working on decoupling the handler from the config though as handlers that were wrapped in middleware were panicking, due to the config not getting set so I'm not sure how one would make the error bubble up to API (which has access to the error handler) without using panic.

A workaround might be to either use the panic handler, or setting the response type to any and returning them as normal responses. If they implement StatusCoder we'd still end up with the correct response code.

jarrettv commented 2 years ago

Here was my simple solution image

abemedia commented 2 years ago

The thing is that we then lose the encoding of different formats. I'm thinking maybe just implement the marshaler types in the error and then in error encoding check if it implements marshaler before using default marshaling.

So to have a custom error that works in all formats you'd implement encoding.TextMarshaler, JSON.Marshaler, XML.Marshaler and yaml.Marshaler.

You'd probably also want to implement don.StatusCoder to give it a custom status code.

I think the simplest way to achieve all of this would be having a StructuredError helper function which you can just pass a struct and a status code to.

abemedia commented 2 years ago

By the way I noticed you're still working on the previous version of Don. I migrated it from standard net/http to using valyala/fasthttp after running some benchmarks and seeing the latter was 10x faster.

jarrettv commented 2 years ago

I went back and forked from net/http as we don't (yet) need the performance of fasthttp. The fasthttp change requires us to wrap/update all our middleware.