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:
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:
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:
Lines 167 to 187 in 838be32
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 theonError
andonSend
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;");
}
- You have a
try { ... } catch (e) { ... }
, this means that you are not followingthe transaction wrapped route handler must not catch any errors internally
, you are catching the error yourself - You are returning an object, not
throw new Error('Account not found')
or similar, hence theonError
will never be called