marcboeker / go-duckdb

go-duckdb provides a database/sql driver for the DuckDB database engine.
MIT License
583 stars 96 forks source link

Proposal: Support better error messages #238

Closed apocelipes closed 2 weeks ago

apocelipes commented 3 weeks ago

Currently, all errors returned from the DuckDB Engine are simply wrapped by errors. New. This makes error handling difficult, and we can't handle it like is common in Golang:

switch err.(type) {
case duckdb.Err1:
    ...
case duckdb.Err2:
    ...

// or
if errors.Is(err, duckdb.Err1) {
    ...
}

However, errors in DuckDB have their own type and a fixed message prefix. As I think, we can wrap all these errors to its own types, like what go-pg/pg and go-sql-driver/mysql does. The code will look like:

type DuckDBError struct {
    // map to DuckDB's ExceptionType
    Type uint8  // we can use uint8 or other suitable types
    Msg string
}

func (e *DuckDBError) Error() string {
    return e.Msg
}

Add a function to get the DuckDBError:

func getDuckdbError(err *C.char) error {
    // find the error prefix
        return &DuckDBError{
                Type: prefix-to-type,
                Msg: C.GoString(err),
        }
}

Error's type can be detected by the prefix of the error message, we can find the mapping here: https://github.com/duckdb/duckdb/blob/main/src/common/exception.cpp#L123

Now we can use this typed error message rather than errors.New, and can handle errors like the idioms in Golang.

Is this change backwards compatible? Yes, DuckDBError will impletment the complete error interface to ensure compatibility.

Will the prefix of the error message be changed in the future? I think prefixes will not be changed, since it is a part of DuckDB's ABI.

I'd like to send a PR to implement it if you guys like this proposal. I also welcome any changes or objections.

apocelipes commented 3 weeks ago

For an example, if I need to insert some data into a table which has a unique constraint:

err := db.ExecuteContext(ctx, "INSERT INTO tb VALUES(...)")
switch derr := err.(*duckdb.DuckDBError); derr.typ {
case duckdb.ErrorTypeConstraint:
    // ignore or do some action when we have a duplicate key
default:
   log.error(derr.Msg)
apocelipes commented 2 weeks ago

I'll close this since the PR has been merged. Feel free to reopen this when any thing ran into problems :)