Certain transaction errors are swallowed

[aslushnikov]

Hi! First and foremost, thank you for the pleasant and capable library!

Issue 1: swallowed errors from manual transactions

Let's say we have the following SQL file with a wrong INSERT statement (types of permissions mismatches the expected one):

-- bad.sql
BEGIN;

CREATE TABLE user_permissions (
  permissions TEXT[] NOT NULL
);

-- Migration script - NOTE that VALUES is broken, so this operation will fail.
INSERT INTO user_permissions (permissions)
VALUES (('read', 'write', 'delete'));

COMMIT;

Trying to execute this file with sql.file silently swallows the error and pretends that everything is good.

// test.mjs
import postgres from 'postgres'

const sql = postgres({ max: 1 });
await sql.file('./bad.sql');
await sql.end();

I'd expect the sql.file command to throw an error.

Issue 2: swallowed errors from sql.begin()

The following also doesn't throw any errors:

import postgres from 'postgres'

const sql = postgres({ max: 1 });

await sql.begin(async sql => [
  sql`
    CREATE TABLE user_permissions (
      permissions TEXT[] NOT NULL
    )
  `,
  sql`
    INSERT INTO user_permissions (permissions)
    VALUES (('read', 'write', 'delete'))
  `,
]);

await sql.end();

Appendix: when the error is correctly thrown

I found that only the following snippet actually throws an error in this case:

import postgres from 'postgres'

const sql = postgres({ max: 1 });

await sql.begin(async sql => {
  await sql`
    CREATE TABLE user_permissions (
      permissions TEXT[] NOT NULL
    )
  `;
  await sql`
    INSERT INTO user_permissions (permissions)
    VALUES (('read', 'write', 'delete'))
  `;
});

await sql.end();

This throws the expected error:

PostgresError: column "permissions" is of type text[] but expression is of type record
    at ErrorResponse (file:///Users/aslushnikov/flakiness/node_modules/postgres/src/connection.js:790:26)
    at handle (file:///Users/aslushnikov/flakiness/node_modules/postgres/src/connection.js:476:6)
    at Socket.data (file:///Users/aslushnikov/flakiness/node_modules/postgres/src/connection.js:315:9)
    at Socket.emit (node:events:514:28)
    at addChunk (node:internal/streams/readable:376:12)
    at readableAddChunk (node:internal/streams/readable:349:9)
    at Readable.push (node:internal/streams/readable:286:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23)
    at cachedError (file:///Users/aslushnikov/flakiness/node_modules/postgres/src/query.js:170:23)
    at new Query (file:///Users/aslushnikov/flakiness/node_modules/postgres/src/query.js:36:24)
    at sql (file:///Users/aslushnikov/flakiness/node_modules/postgres/src/index.js:112:11)
    at file:///Users/aslushnikov/flakiness/database/test.mjs:18:12 {

Is this a known issue? Do I do something wrong?

[porsager]

Hi. Great error report! Thank you!

I think your expectations are correct, so that's quite the bug you've found here. I'll dig into it.

[porsager]

Ok, I found the issue. It was accidentally trying to use the routineRetry logic for simple queries (should only be used for prepared statements), which makes no sense, and messes with the protocol expectations for simple queries.

You can try it out by installing latest master using npm i porsager/postgres

Related to your issue 2, you have a stray "async" for your function. The function should look like this:

import postgres from 'postgres'

const sql = postgres({ max: 1 });

await sql.begin(sql => [
  sql`
    CREATE TABLE user_permissions (
      permissions TEXT[] NOT NULL
    )
  `,
  sql`
    INSERT INTO user_permissions (permissions)
    VALUES (('read', 'write', 'delete'))
  `,
]);

await sql.end();

[aslushnikov]

This is awesome, thank you for the quick fix!

[aslushnikov]

You can try it out by installing latest master using npm i porsager/postgres

@porsager I just tried, but the issue still reproduces for me on tip-of-tree. I tried cloning & building the repository, but no luck!

[porsager]

Didn't transpile for cjs etc, so if you're not using the esm version that might be it. Just a sec 😊

[porsager]

Try again now 😉

[porsager]

If there's still an error, do post it 👍

[aslushnikov]

Still no luck; here's the exact thing that I try: https://github.com/aslushnikov/postgres-js-issue-repro

To validate the repro, there's a commented-out node-postgres code snippet that does throw an error:

https://github.com/aslushnikov/postgres-js-issue-repro/blob/efac21d154e790334a31c93e36cd4bb9241441bf/test.mjs#L16-L21

[aslushnikov]

To make sure this is not something specific to my machine, here are Github Actions running this test: https://github.com/aslushnikov/postgres-js-issue-repro/actions/runs/8364618788/job/22900381583

[porsager]

There we go :-) Single character mistake - sorry

[aslushnikov]

@porsager awesome, it works now!

I had to build it manually since installation from GH doesn't work. That's not an issue for me though – i'll happily wait for the next release.

Thank you!