Configure proteus to panic on database errors

[SharkFourSix]

This PR adds the ability to configure proteus to panic (or not) when the underlying database returns an error.

This is especially useful since we have the ability to chose whether a DAO function returns an error or not. In such a case where the error return type is omitted, there would be no way of knowing if the database returned an error. At the same time, reducing the amount of boilerplate code to check for errors that may actually never exist.

Configuration is done by passing three values in the build context.

• Introduced a new type ContextKey as key type for passing values in the builder context
• Introduced a new type ErrorBehavior to control the said scenarios.

ErrorBehavior Values:

• DoNothing (Default) - proteus does not do anything when the underlying data source throws an error.
• PanicWhenAbsent - proteus will panic, only if the DAO function being called does not have the error return type.
• PanicAlways - proteus will always panic if there is an error from the data source, whether or not the DAO function being called indicates an error return type.

[jonbodner]

I'm going to have to think about this. I don't like the idea of panics being used as a way to propagate errors.

[jonbodner]

Current thinking: PanicAlways mode is a bad idea. If a function doesn't have an error return value, the panic on error behavior might be better behavior. The existing behavior that swallows errors is probably a mistake. I don't know if users should get to choose between swallowing errors and panicking.

[SharkFourSix]

Current thinking: PanicAlways mode is a bad idea. If a function doesn't have an error return value, the panic on error behavior might be better behavior. The existing behavior that swallows errors is probably a mistake. I don't know if users should get to choose between swallowing errors and panicking.

I understand. These use cases are legit. That is why they need to be configurable. By default, the errors are swallowed, the way you designed it. The caller still has the option to get the errors back through the return type.

For my uses cases, this is how I actually design my database calls. I usually do the following:

type userService struct {
    db * sqlx.DB
}

func (svc *userService) FindById(id int) *models.User {
    var users []*models.Users

    query := "select * from users where id = $1"

    err := svc.db.Query(&users, query, id)
    if err != nil {
        panic(errors.Wrap(err, "error fetching user"))
    }

    if len(users) > 0 {
        return users[0]
    }

    return nil
}

See, this way during development when that function is tested and doesn't panic, I know and I'm confident that the query and the database environment are properly set-up.

If there's a panic, there is a much bigger issue that needs to be handled.

Up the process pipeline, I then have a function to recover the error. If it's within an http server context, I'll log the stack trace and error, and then return a 500 internal server error. If the app is configured in debug mode, I'll also return the error to the client.

It helps very much. With proteus, my "service" calls would have ended up with having to do what I indicated above. At the moment as I wait for the PR I've resorted to this:

func PanicOnError[T any](val T, err error) T {
    if err := nil {
        panic(errors.Wrap(err, "fatal error in database call"))
    }
    return val
}

UserDao {
    FindById func(...) (*User, error) `proq:"...."`
}

user := PanicOnError(userDao.FindById(..., ..., id))

I really like the concept of proteus and I do have some more ideas that I'd like to contribute. So please consider these uses cases. They are legitimate.

[jonbodner]

I actually like that helper function quite a bit. Here's a function that extends it a bit to wrap all of the function fields in a struct that match the pattern of one return value and one returned error:

import (
    "fmt"
    "reflect"
)

var errorType = reflect.TypeOf((*error)(nil)).Elem()

func ErrorsToPanics[T any](dao T) T {
    dt := reflect.TypeOf(dao)
    if dt.Kind() != reflect.Struct {
        panic("dao must be a struct")
    }
    dv := reflect.ValueOf(dao)
    out := reflect.New(dt).Elem()
    for i := 0; i < dv.NumField(); i++ {
        curField := dv.Field(i)
        out.Field(i).Set(curField)
        if curField.Kind() == reflect.Func {
            if curField.Type().NumOut() == 2 && curField.Type().Out(1).Implements(errorType) {
                out.Field(i).Set(reflect.MakeFunc(curField.Type(), func(args []reflect.Value) []reflect.Value {
                    result := curField.Call(args)
                    if !result[1].IsNil() {
                        panic(fmt.Errorf("fatal error in database call: %w", result[1].Interface().(error)))
                    }
                    return result
                }))
            }
        }
    }
    return out.Interface().(T)
}

You can use it like this:

import (
    "fmt"
    "testing"
)

func TestErrorsToPanics(t *testing.T) {
    type testStruct struct {
        DoSomething     func() (string, error)
        DoSomethingElse func(i int) string
        AnotherField    int
    }
    var ts testStruct
    ts.DoSomething = func() (string, error) {
        return "hello", nil
    }
    ts.DoSomethingElse = func(i int) string {
        return fmt.Sprintf("%d", i)
    }

    result, _ := ts.DoSomething()
    fmt.Println(result)
    fmt.Println(ts.DoSomethingElse(5))
    ts = ErrorsToPanics(ts)
    result, _ = ts.DoSomething()
    fmt.Println(result)
    fmt.Println(ts.DoSomethingElse(5))

    ts.DoSomething = func() (string, error) {
        return "", fmt.Errorf("test error")
    }
    ts = ErrorsToPanics(ts)

    func() {
        defer func() {
            if r := recover(); r == nil {
                t.Error("Expected panic")
            }
        }()
        result, _ = ts.DoSomething()
        t.Error("should have panicked")
    }()

    func() {
        defer func() {
            if r := recover(); r == nil {
                t.Error("Expected panic")
            }
        }()
        var i int
        i = ErrorsToPanics(i)
        t.Error("should have panicked")
    }()
}

It's not as clean, since you have to assign the error to _, but it gives you the panic.

I'm hesitant to have proteus return functions that panic, because I don't think it's a great idea for a panic to cross a library boundary. But I'm still thinking about it.

[jonbodner]

Also see https://google.github.io/styleguide/go/decisions#dont-panic and https://google.github.io/styleguide/go/best-practices#program-checks-and-panics for Google's advice on when to use panic.