encore
encore copied to clipboard
encore.dev/storage/sqldb does not expose PostgreSQL error codes
In order to detect various PostgreSQL errors it would be great if Encore's sqldb package exposed the underlying error code.
Workarounds exist (see #117) but they rely on the underlying database driver being jackc/pgx
. Encore should expose these error codes natively through the sqldb
package so that applications do not need to poke at the underlying database driver.
@eandre I'm poking around looking for a potential hacktoberfest issue. I've read the contribution guidelines but, I didn't see a section on how your team assigns issues to contributors. Do you request that we just open a PR and tag the issue or is there an assignment process?
Hey @ElliottDenlinger, great question! For this issue, since it requires some design work (designing what the API should look like, before coding up the implementation) it's probably easiest to first assign the issue so we don't have multiple people scrambling to take on the same task.
If you're interested in working on this let me know and we can assign this to you :) Thank for your expressing interest!
@eandre I'd love to at least take a crack at it. I've not read any of the encore source other than a quick skim this morning so we'll see if I'm in over my head or not :D.
I'm not too familiar with your current strategy for chaining errors. Did you have any immediate thoughts on what you were hoping to see as a way to expose the codes? I'll go spelunking through the way you are currently wrapping / converting errors and see if anything comes to mind.
I've had a change of plans come up for the next few weeks in the time since we started discussing my taking the issue. Others should feel free to ask for this to be assigned to them.
@eandre I'll take a look at this over the weekend if no one else is currently working on it.
Sure, thanks @ronaudinho!
did a bit of digging and so far I see several ways to go about this:
- wrapping
pgconn.Error
as an error type that implements errs.ErrDetails interface -> would require two types assertion to unwrap as I see it, which of course can be done in a new convenient function - if it is for internal use, perhaps in can be included Meta field inside errs.Error -> as I understand it, the underlying error code is to be exposed, so this does not seem right
- add a field in
errs.Error
for this -> simplest, but feels weird to me whichever chosen process would be done in convertErr and personally, I lean towards option 1, but it seems rather complex and perhaps I am missing something, thoughts?
in terms of exposing the error codes, the options i can think of are:
- adding them in the errs package or perhaps in this
sqldb
(with some codegen to avoid repetitive works of adding new error codes perhaps) - let user get them from pgerrcode
- leaving it as is and let user do what they want with it
The errs
package is primarily for API errors, and while the sqldb
errors currently return *errs.Error
objects I think that was a design mistake.
My gut feeling is that we should add an *sqldb.PgError
type similar to https://pkg.go.dev/github.com/jackc/pgconn#PgError (but we can leave out a bunch of the details for now, I think). I don't think we need to define all the error code constants; they're already well-documented and won't change.
Thoughts on this approach?
sounds about right to me. to clarify, does this mean changing the type of error returned by convertErr
function from *errs.Error
into this new *sqldb.PgError
?