go-don icon indicating copy to clipboard operation
go-don copied to clipboard

Consider rfc7807 error response handling

Open jarrettv opened this issue 2 years ago • 6 comments

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 avatar Mar 20 '22 18:03 jarrettv

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

jarrettv avatar Mar 21 '22 20:03 jarrettv

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.

abemedia avatar Mar 22 '22 18:03 abemedia

Here was my simple solution image

jarrettv avatar Mar 24 '22 02:03 jarrettv

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 avatar Mar 24 '22 18:03 abemedia

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.

abemedia avatar Mar 24 '22 18:03 abemedia

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.

jarrettv avatar May 24 '22 12:05 jarrettv