fastify/fastify-postgres

Documented Transactions doesnt work

Closed this issue · 5 comments

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.26.2

Plugin version

5.2.2

Node.js version

v18.20.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

12

Description

I am working on a file structure consisting of server.js, route.js and profile.js and am trying to implement a PATCH request within profile.js to save data into the database.
Originally, I had the following code:

fastify.patch('/', {pg: {transact: true}}, async (request) => {
// Code
}

And attempted to implement a query:

try {
    // ...
    const edit = await request.pg.query(`UPDATE wallets
                                         SET username = $1
                                         WHERE phonenumber = $2
                                         AND useractive = true;`, [requestBody.username, accountId]);
    if (edit.rowCount === 0) {
        return {
            success: false,
            error: [
                {
                    message: "Account not found",
                    code: 400
                }
            ]
        }
    }
    // ...
} catch (e) {
    await request.pg.query("ROLLBACK;");
}

However, I am encountering a TypeError: Cannot read properties of null (reading 'query').
I've tried various alternatives, such as renaming {pg: {transact: true}} to {pg: {transact: 'accEdit'}} and changing the query structure from request.pg.query(...) to request.pg['accEdit'].query(...).
Despite these changes, I'm either receiving no errors from fastify-pg (though the test fails and changes are still made) or I'm getting an error stating that fastify.transact doesn't exist.
In addition to assistance with these errors, I would appreciate a practical documentation with Error Handling.

Steps to Reproduce

'use strict'

/**
 *
 * @param fastify
 * @return {Promise<void>}
 */
const editAccount = async (fastify) => {
    fastify.patch('/', {pg: {transact: 'accEdit'}}, async (request) => {
        try {
            const param = request.params.accountId;
            const authKey = request.headers["authorization"] || null;
            const authUsername = request.headers["x-auth-username"] || null;
            const requestBody = request.body || null;
            if (!authKey || !authUsername || !param || !requestBody) {
                return {
                    success: false,
                    error: [
                        {
                            message: "Account not found",
                            code: 400
                        }
                    ]
                };

            const edit = await request.pg.query(`UPDATE wallets
                                                 SET username = $1
                                                 WHERE phonenumber = $2
                                                   AND accountactive = true;`, [requestBody.username, accountId]);
            if (edit.rowCount === 0) {
                return {
                    success: false,
                    error: [
                        {
                            message: "Account not found",
                            code: 400
                        }
                    ]
                }
            }
            fastify.done(); // Thats the thing that makes the test error
            await request.pg.query("COMMIT;")
        } catch (e) {
            await fastify.pg.query("ROLLBACK;");
            fastify.log.error("There was an error updating the Data!\n"+e)

        } finally {
            await fastify.pg.release;
        }
    })
}

module.exports = editAccount;

Expected Behavior

It should get the error and handle it and not commit it to the Database

There are tests for this functionality:

t.test('Should be able to execute queries provided to the request pg decorator', async t => {

Either supply a way to reproduce your issue locally (Stackblitz, GitHub repo or something) or even better: Make a PR where you extend req-initialization.test.js to prove the alleged error, since that will anyhow need to be created to fix any issue.

There are tests for this functionality:

t.test('Should be able to execute queries provided to the request pg decorator', async t => {

Either supply a way to reproduce your issue locally (Stackblitz, GitHub repo or something) or even better: Make a PR where you extend req-initialization.test.js to prove the alleged error, since that will anyhow need to be created to fix any issue.

The thing is I'm not allowed to make the code public. I just can show the code in private.

The thing is I'm not allowed to make the code public. I just can show the code in private.

Are you allowed to make a PR like I suggested? I would say that as it stands I think this issue will be quite hard to move forward with, it essentially amounts to a help request rather than a bug report.

The thing is I'm not allowed to make the code public. I just can show the code in private.

Are you allowed to make a PR like I suggested? I would say that as it stands I think this issue will be quite hard to move forward with, it essentially amounts to a help request rather than a bug report.

Not really because we dont use unit tests and its also a completely different file and more

The rollback happens on the onError callback, see:

fastify-postgres/index.js

Lines 167 to 187 in 838be32

const onError = (req, reply, error, done) => {
req[transactionFailedSymbol] = true
extractRequestClient(req, transact).query('ROLLBACK', done)
}
const onSend = async (req) => {
const requestClient = extractRequestClient(req, transact)
if (requestClient) {
try {
if (!req[transactionFailedSymbol]) {
await requestClient.query('COMMIT')
}
} finally {
requestClient.release()
}
}
}
routeOptions.preHandler = addHandler(routeOptions.preHandler, preHandler)
routeOptions.onError = addHandler(routeOptions.onError, onError)
routeOptions.onSend = addHandler(routeOptions.onSend, onSend)

This is described in Transact route option:

Important: rolling back a transaction relies on the handler failing and being caught by an onError hook. This means that the transaction wrapped route handler must not catch any errors internally.

In the plugin this works by using the preHandler hook to open the transaction, then the onError and onSend hooks to commit or rollback and release the client back to the pool.

I'm a bit rusty on this but as far as I can see you have two things wrong:

try {
    // ...
    const edit = await request.pg.query(`UPDATE wallets
                                         SET username = $1
                                         WHERE phonenumber = $2
                                         AND useractive = true;`, [requestBody.username, accountId]);
    if (edit.rowCount === 0) {
        return {
            success: false,
            error: [
                {
                    message: "Account not found",
                    code: 400
                }
            ]
        }
    }
    // ...
} catch (e) {
    await request.pg.query("ROLLBACK;");
}
  1. You have a try { ... } catch (e) { ... }, this means that you are not following the transaction wrapped route handler must not catch any errors internally, you are catching the error yourself
  2. You are returning an object, not throw new Error('Account not found') or similar, hence the onError will never be called