dbos-inc / dbos-transact-ts

The TypeScript framework for backends that scale
https://docs.dbos.dev
MIT License
373 stars 28 forks source link

Stored Procedures Swallow Errors #626

Closed kraftp closed 1 month ago

kraftp commented 2 months ago

Original context from jansommer on Discord:

I have the following @Transaction:

@Transaction()
static async signup(ctx: TransactionContext<Knex>, input: SignupInput): Promise<SignoutOutut> {
    try {
        const _todo = await ctx.client.raw(`select signup(:email, :password)`, input) as unknown
        return { ok: true }
    } catch (err) {
        return { ok: false, error: "todo" }
    }
}

It runs this function:

create or replace function signup(email text, password text, out session session)
returns session as $$
declare
  user_id bigint;
begin
  -- https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html#implement-proper-password-strength-controls
  if length(password) < 8 then
    raise 'password_too_short' using
      errcode = 'RX400',
      hint = 'Password must be >= 8 characters long';
  end if;

  ...
exception
  when unique_violation then
    raise 'user_exists' using errcode = 'RX409';
end
$$ language plpgsql volatile;

If no exception is raised, the user is signed up, but the exception gets swallowed by DBOS if one is raised:

2024-09-23 11:47:47 [error]: INSERT INTO dbos.transaction_outputs (workflow_uuid, function_id, output, txn_id, txn_snapshot, created_at) VALUES ($1, $2, $3, (select pg_current_xact_id_if_assigned()::text), $4, $5) RETURNING txn_id; - current transaction is aborted, commands ignored until end of transaction block 
    at Parser.parseErrorMessage (/home/jan/Clients/restyx/v2/restyx/apps/backend/node_modules/pg-protocol/dist/parser.js:283:98)
    at Parser.handlePacket (/home/jan/Clients/restyx/v2/restyx/apps/backend/node_modules/pg-protocol/dist/parser.js:122:29)
    at Parser.parse (/home/jan/Clients/restyx/v2/restyx/apps/backend/node_modules/pg-protocol/dist/parser.js:35:38)
    at Socket.<anonymous> (/home/jan/Clients/restyx/v2/restyx/apps/backend/node_modules/pg-protocol/dist/index.js:11:42)
    at Socket.emit (node:events:519:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
    at Readable.push (node:internal/streams/readable:390:5)
    at TCP.onStreamRead (node:internal/stream_base_commons:191:23)

And here's the contents of the error:

{
  length: 145,
  severity: 'ERROR',
  code: '25P02',
  detail: undefined,
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'postgres.c',
  line: '1496',
  routine: 'exec_parse_message',
  dbos_already_logged: true
}
devhawk commented 2 months ago

I wrote a test to try and replicate this issue, but DBOS persisted the error in tx output table as expected.

devhawk commented 1 month ago

jansommer provided a repro. In his repro, he has a DB function that raises an error:

create function xx() returns void as $$
begin raise 'password_too_short'; end
$$ language plpgsql;

and a @Transaction method that calls it

@Transaction()
static async root(ctxt: TransactionContext<Knex>) {
    return await ctxt.client.raw("select xx()")
}

so far, so good. However, if he tries to catch the exception in the method, we fail to save the transaction output because the raise statement aborted the db transaction.

{
  "status": 500,
  "message": "INSERT INTO dbos.transaction_outputs (workflow_uuid, function_id, output, txn_id, txn_snapshot, created_at) VALUES ($1, $2, $3, (select pg_current_xact_id_if_assigned()::text), $4, $5) RETURNING txn_id; - current transaction is aborted, commands ignored until end of transaction block",
  "details": {
    "length": 145,
    "name": "error",
    "severity": "ERROR",
    "code": "25P02",
    "file": "postgres.c",
    "line": "1498",
    "routine": "exec_parse_message",
    "dbos_already_logged": true
  }
}