pg: More verbose error traces

Partly because some errors currently appeared to come from within FU::PG
itself, which is useless, and partly because it's common to wrap
database access methods, while that's exactly the kind of operation
where you *really* want to know where the error originated from.

(Source: too much time wasted debugging VNDB errors)
This commit is contained in:
Yorhel 2025-02-07 10:59:27 +01:00
parent 96aee880ce
commit 7c8473533d
4 changed files with 46 additions and 27 deletions

8
FU.xs
View file

@ -15,10 +15,10 @@
#define FUPG_CONN_COOKIE \ #define FUPG_CONN_COOKIE \
if (c->cookie) croak("Invalid attempt to run a query on the top-level connection while a transaction object exists") if (c->cookie) fu_confess("Invalid attempt to run a query on the top-level connection while a transaction object exists")
#define FUPG_ST_COOKIE \ #define FUPG_ST_COOKIE \
if (st->cookie != st->conn->cookie) croak("Invalid cross-transaction operation on statement object") if (st->cookie != st->conn->cookie) fu_confess("Invalid cross-transaction operation on statement object")
typedef fupg_conn *fupg_txn; typedef fupg_conn *fupg_txn;
@ -37,14 +37,14 @@ fupg_st * FUPG_ST
INPUT INPUT
FUPG_CONN FUPG_CONN
if (sv_derived_from($arg, \"FU::PG::conn\")) $var = (fupg_conn *)SvIVX(SvRV($arg)); if (sv_derived_from($arg, \"FU::PG::conn\")) $var = (fupg_conn *)SvIVX(SvRV($arg));
else croak(\"invalid connection object\"); else fu_confess(\"invalid connection object\");
FUPG_TXN FUPG_TXN
$var = fupg_get_transaction(aTHX_ $arg); $var = fupg_get_transaction(aTHX_ $arg);
FUPG_ST FUPG_ST
if (sv_derived_from($arg, \"FU::PG::st\")) $var = (fupg_st *)SvIVX(SvRV($arg)); if (sv_derived_from($arg, \"FU::PG::st\")) $var = (fupg_st *)SvIVX(SvRV($arg));
else croak(\"invalid statement object\"); else fu_confess(\"invalid statement object\");
#" #"
EOT EOT

View file

@ -15,7 +15,7 @@ package FU::PG::conn {
}; };
package FU::PG::txn { package FU::PG::txn {
use Carp 'croak'; use Carp 'confess';
my $COUNTER = 0; my $COUNTER = 0;
@ -27,19 +27,19 @@ package FU::PG::txn {
# 2: $parent, undef if this is a top-level transaction. # 2: $parent, undef if this is a top-level transaction.
sub commit($t) { sub commit($t) {
croak "Unable to commit transaction that has already finished" if !$t->[1]; confess "Unable to commit transaction that has already finished" if !$t->[1];
$t->exec($t->[2] ? "RELEASE SAVEPOINT fupg_$t->[1]" : 'COMMIT'); $t->exec($t->[2] ? "RELEASE SAVEPOINT fupg_$t->[1]" : 'COMMIT');
$t->[1] = undef; $t->[1] = undef;
} }
sub rollback($t) { sub rollback($t) {
croak "Unable to rollback transaction that has already finished" if !$t->[1]; confess "Unable to rollback transaction that has already finished" if !$t->[1];
$t->exec($t->[2] ? "ROLLBACK TO SAVEPOINT fupg_$t->[1]" : 'ROLLBACK'); $t->exec($t->[2] ? "ROLLBACK TO SAVEPOINT fupg_$t->[1]" : 'ROLLBACK');
$t->[1] = undef; $t->[1] = undef;
} }
sub txn($t) { sub txn($t) {
croak "Unable to create sub-transaction when current transaction has already finished" if !$t->[1]; confess "Unable to create sub-transaction when current transaction has already finished" if !$t->[1];
$COUNTER++; $COUNTER++;
$t->exec("SAVEPOINT fupg_$COUNTER"); $t->exec("SAVEPOINT fupg_$COUNTER");
$t->[0]->_set_cookie($COUNTER); $t->[0]->_set_cookie($COUNTER);
@ -80,7 +80,7 @@ FU::PG - Another PostgreSQL client module
$conn->q('INSERT INTO books (title) VALUES ($1)', 'Revelation Space')->exec; $conn->q('INSERT INTO books (title) VALUES ($1)', 'Revelation Space')->exec;
for my ($id, $title) ($conn->q('SELECT * FROM books')->flat) { for my ($id, $title) ($conn->q('SELECT * FROM books')->flat->@*) {
print "$id: $title\n"; print "$id: $title\n";
} }
@ -365,8 +365,6 @@ handling, that is still available:
Just don't try to use transaction objects and manual transaction commands at Just don't try to use transaction objects and manual transaction commands at
the same time, that won't end well. the same time, that won't end well.
=back
=head2 Errors =head2 Errors

View file

@ -26,7 +26,7 @@ static SV *fu_croak_hv(HV *hv, const char *klass, const char *message, ...) {
ENTER; ENTER;
SAVETMPS; SAVETMPS;
PUSHMARK(SP); PUSHMARK(SP);
XPUSHs(sv_2mortal(sv)); mXPUSHs(sv);
PUTBACK; PUTBACK;
call_pv("Carp::longmess", G_SCALAR); call_pv("Carp::longmess", G_SCALAR);
hv_stores(hv, "full_message", SvREFCNT_inc(POPs)); hv_stores(hv, "full_message", SvREFCNT_inc(POPs));
@ -36,6 +36,27 @@ static SV *fu_croak_hv(HV *hv, const char *klass, const char *message, ...) {
return sv_bless(sv_2mortal(newRV_noinc((SV *)hv)), gv_stashpv(klass, GV_ADD)); return sv_bless(sv_2mortal(newRV_noinc((SV *)hv)), gv_stashpv(klass, GV_ADD));
} }
__attribute__((noreturn, format (printf, 1, 2)))
static void fu_confess(const char *message, ...) {
va_list args;
SV *sv;
dTHX;
dSP;
va_start(args, message);
sv = vnewSVpvf(message, &args);
va_end(args);
ENTER;
SAVETMPS;
PUSHMARK(SP);
mXPUSHs(sv);
PUTBACK;
call_pv("Carp::confess", G_DISCARD);
/* Won't happen, but a safe fallback */
croak("%s", SvPV_nolen(sv));
}
/* Custom string builder, should be slightly faster than using Sv* macros directly. */ /* Custom string builder, should be slightly faster than using Sv* macros directly. */

View file

@ -142,11 +142,11 @@ static fupg_conn *fupg_get_transaction(pTHX_ SV *sv) {
v = av_fetch(av, 1, 0); v = av_fetch(av, 1, 0);
if (!v || !*v) goto invalid; if (!v || !*v) goto invalid;
if (!SvOK(*v)) croak("Invalid attempt to run a query on a transaction that has already finished"); if (!SvOK(*v)) fu_confess("Invalid attempt to run a query on a transaction that has already finished");
if (c->cookie != SvUV(*v)) croak("Invalid cross-transaction operation"); if (c->cookie != SvUV(*v)) fu_confess("Invalid cross-transaction operation");
return c; return c;
invalid: invalid:
croak("invalid transaction object"); fu_confess("invalid transaction object");
} }
/* Read a Perl value from a PGresult. /* Read a Perl value from a PGresult.
@ -196,7 +196,7 @@ static SV *fupg_q(pTHX_ fupg_conn *c, const char *query, I32 ax, I32 argc) {
static void fupg_st_prepare(pTHX_ fupg_st *st) { static void fupg_st_prepare(pTHX_ fupg_st *st) {
if (st->describe) return; if (st->describe) return;
if (st->prepared) croak("invalid attempt to re-prepare invalid statement"); if (st->prepared) fu_confess("invalid attempt to re-prepare invalid statement");
/* TODO: This is where we check for any cached prepared statements */ /* TODO: This is where we check for any cached prepared statements */
@ -253,7 +253,7 @@ static void fupg_st_check_dupcols(pTHX_ PGresult *r) {
int len = -strlen(key); int len = -strlen(key);
if (hv_exists(hv, key, len)) { if (hv_exists(hv, key, len)) {
SvREFCNT_dec((SV *)hv); SvREFCNT_dec((SV *)hv);
croak("Query returns multiple columns with the same name ('%s')", key); fu_confess("Query returns multiple columns with the same name ('%s')", key);
} }
hv_store(hv, key, len, &PL_sv_yes, 0); hv_store(hv, key, len, &PL_sv_yes, 0);
} }
@ -264,7 +264,7 @@ static void fupg_st_execute(pTHX_ fupg_st *st) {
/* Disallow fetching the results more than once. I don't see a reason why /* Disallow fetching the results more than once. I don't see a reason why
* someone would need that and disallowing it leaves room for fetching the * someone would need that and disallowing it leaves room for fetching the
* results in a streaming fashion without breaking API compat. */ * results in a streaming fashion without breaking API compat. */
if (st->result) croak("Invalid attempt to execute statement multiple times"); if (st->result) fu_confess("Invalid attempt to execute statement multiple times");
/* TODO: prepare can be skipped when prepared statement caching is disabled and (text-format queries or no bind params) */ /* TODO: prepare can be skipped when prepared statement caching is disabled and (text-format queries or no bind params) */
fupg_st_prepare(aTHX_ st); fupg_st_prepare(aTHX_ st);
@ -302,10 +302,10 @@ static SV *fupg_st_exec(pTHX_ fupg_st *st) {
static SV *fupg_st_val(pTHX_ fupg_st *st) { static SV *fupg_st_val(pTHX_ fupg_st *st) {
fupg_st_prepare(aTHX_ st); fupg_st_prepare(aTHX_ st);
if (PQnfields(st->describe) > 1) croak("Invalid use of $st->val() on query returning more than one column"); if (PQnfields(st->describe) > 1) fu_confess("Invalid use of $st->val() on query returning more than one column");
if (PQnfields(st->describe) == 0) croak("Invalid use of $st->val() on query returning no data"); if (PQnfields(st->describe) == 0) fu_confess("Invalid use of $st->val() on query returning no data");
fupg_st_execute(aTHX_ st); fupg_st_execute(aTHX_ st);
if (PQntuples(st->result) > 1) croak("Invalid use of $st->val() on query returning more than one row"); if (PQntuples(st->result) > 1) fu_confess("Invalid use of $st->val() on query returning more than one row");
SV *sv = PQntuples(st->result) == 0 ? newSV(0) : fupg_val(aTHX_ st->result, 0, 0); SV *sv = PQntuples(st->result) == 0 ? newSV(0) : fupg_val(aTHX_ st->result, 0, 0);
return sv_2mortal(sv); return sv_2mortal(sv);
} }
@ -313,8 +313,8 @@ static SV *fupg_st_val(pTHX_ fupg_st *st) {
static I32 fupg_st_rowl(pTHX_ fupg_st *st, I32 ax) { static I32 fupg_st_rowl(pTHX_ fupg_st *st, I32 ax) {
dSP; dSP;
fupg_st_execute(aTHX_ st); fupg_st_execute(aTHX_ st);
if (PQntuples(st->result) == 0) croak("Invalid use of $st->rowl() on query returning zero rows"); if (PQntuples(st->result) == 0) fu_confess("Invalid use of $st->rowl() on query returning zero rows");
if (PQntuples(st->result) > 1) croak("Invalid use of $st->rowl() on query returning more than one row"); if (PQntuples(st->result) > 1) fu_confess("Invalid use of $st->rowl() on query returning more than one row");
if (GIMME_V != G_LIST) { if (GIMME_V != G_LIST) {
ST(0) = sv_2mortal(newSViv(PQnfields(st->result))); ST(0) = sv_2mortal(newSViv(PQnfields(st->result)));
return 1; return 1;
@ -328,8 +328,8 @@ static I32 fupg_st_rowl(pTHX_ fupg_st *st, I32 ax) {
static SV *fupg_st_rowa(pTHX_ fupg_st *st) { static SV *fupg_st_rowa(pTHX_ fupg_st *st) {
fupg_st_execute(aTHX_ st); fupg_st_execute(aTHX_ st);
if (PQntuples(st->result) == 0) croak("Invalid use of $st->rowl() on query returning zero rows"); if (PQntuples(st->result) == 0) fu_confess("Invalid use of $st->rowl() on query returning zero rows");
if (PQntuples(st->result) > 1) croak("Invalid use of $st->rowl() on query returning more than one row"); if (PQntuples(st->result) > 1) fu_confess("Invalid use of $st->rowl() on query returning more than one row");
int i, nfields = PQnfields(st->result); int i, nfields = PQnfields(st->result);
AV *av = newAV_alloc_x(nfields); AV *av = newAV_alloc_x(nfields);
SV *sv = sv_2mortal(newRV_noinc((SV *)av)); SV *sv = sv_2mortal(newRV_noinc((SV *)av));
@ -341,8 +341,8 @@ static SV *fupg_st_rowh(pTHX_ fupg_st *st) {
fupg_st_prepare(aTHX_ st); fupg_st_prepare(aTHX_ st);
fupg_st_check_dupcols(aTHX_ st->describe); fupg_st_check_dupcols(aTHX_ st->describe);
fupg_st_execute(aTHX_ st); fupg_st_execute(aTHX_ st);
if (PQntuples(st->result) == 0) croak("Invalid use of $st->rowh() on query returning zero rows"); if (PQntuples(st->result) == 0) fu_confess("Invalid use of $st->rowh() on query returning zero rows");
if (PQntuples(st->result) > 1) croak("Invalid use of $st->rowh() on query returning more than one row"); if (PQntuples(st->result) > 1) fu_confess("Invalid use of $st->rowh() on query returning more than one row");
int i, nfields = PQnfields(st->result); int i, nfields = PQnfields(st->result);
HV *hv = newHV(); HV *hv = newHV();
SV *sv = sv_2mortal(newRV_noinc((SV *)hv)); SV *sv = sv_2mortal(newRV_noinc((SV *)hv));