georgysavva / scany

Library for scanning data from a database into Go structs and more
MIT License
1.27k stars 67 forks source link

Error handling #13

Closed jfyne closed 4 years ago

jfyne commented 4 years ago

Hi thanks for the library.

Can you explain the best way to handle a pgx.ErrNoRows? It appears I can't just do something like


if err := pgxscan.Select(ctx, db, "select * from whatever"); err != nil {
    // note this is the standard import "errors"
    if errors.Is(err, pgx.ErrNoRows) {
        return fmt.Errorf("no results: %w", err)
    }
    return err
}

Does this mean I have to use "pkg/errors" to find the cause?

Thanks

georgysavva commented 4 years ago

Hi. Thanks for reaching out.

The only case where pgx return such error pgx.ErrNoRows is .QueryRow().Scan():

if err:=db.QueryRow(ctx, "select * from whatever limit 1").Scan(&result); err!=nil {
        if errors.Is(err, pgx.ErrNoRows) {
             return fmt.Errorf("no results: %w", err)
    }
}

It measn that you expect exactly 1 record. To achieve the same behavior with pgxscan, you need to use pgxscan.Get() and later a convenient utillity function pgxscan.NotFound() that check is it not found error or not:

if err:=pgxscan.Get(ctx, db, &result, "select * from whatever limit 1"); err!=nil {
        if pgxscan.NotFound(err) {
             return fmt.Errorf("no results: %w", err)
    }
}

pgxscan.Select() doesn't return such error at all, because it's intended to select multiple rows and it's not an error if there is zero rows. But you can always check the length of result slice and if it's zero it means that there is no rows.

Let me know if you have any questions left

jfyne commented 4 years ago

Ah yes I see, thank you. I accidentally put Select instead of Get. That helper function is exactly what I need.

Have you considered replacing github.com/pkg/errors with the stdlib error wrapping?

georgysavva commented 4 years ago

As far as I know, github.com/pkg/errors is compatible with stdlib error wrapping. So you can easily use stdlib errors.Is() on errors that were wrapped by pkg/errors. Apart from that pkg/errors contains convenient errors.WithStack() wrapper that stdlib is missing. So I don't see any benefits in switching to stdlib errors package. Or I missing something?

jfyne commented 4 years ago

I ask that because I was originally trying to handle the error from Get using this library function to wrap any sql specific errors. And I was expecting pgxscan to just return the underlying pgx.ErrNoRows. I found that the first case statement wasn't matching. I was just getting a "scany: no row was found" error.

// Handles sql errors and converts to a grpc specific error with code. Returns error and if it was handled or not.
func sql2grpc(err error) (error, bool) {
    var pgErr *pgconn.PgError

    switch {
    case errors.Is(err, sql.ErrNoRows), errors.Is(err, pgx.ErrNoRows):
        return status.Errorf(codes.NotFound, ErrNotFound.Error()), true
    case errors.As(err, &pgErr):
        switch pgErr.Code {
        case pgerrcode.UniqueViolation:
            return status.Errorf(codes.AlreadyExists, ErrAlreadyExists.Error()), true
        }
    }
    return err, false
}

Adding the additional pgxscan.NotFound(err) check works though.

    case errors.Is(err, sql.ErrNoRows), errors.Is(err, pgx.ErrNoRows), pgxscan.NotFound(err):
georgysavva commented 4 years ago

Yeah, The reason for this is that pgxscan doesn't call pgx.QueryRow().Scan() at all, in any circumstances. This is due to the fact that it always needs pgx.Rows, and pgx.Row (returned by pgx.QueryRow()) won't work. And because of this pgxscan implements zero rows handling on its own and return its own err type.

So adding case with pgxscan.NotFound(err) is the way to go.

jfyne commented 4 years ago

Ok great thanks.

Would making a special case in pgxscan to return pgx.ErrNoRows work? And likewise in sqlscan a sql.ErrNoRows case? Might make the library more plug and play.

Either way thanks for helping me out!

georgysavva commented 4 years ago

Actually it's a good idea. pgxscan and sqlscan use abstract dbscan package that returns its own errNotFound error, they could check this error by dbscan.NotFound() and replace it with pgx.ErrNoRows and sql.ErrNoRows correspondingly.

I will implement this in my spare time or you could shout a PR if you like.

jfyne commented 4 years ago

I'll do a PR for you to review