Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

originalError undefined in some transaction errors #1716

Open
Angelelz opened this issue Nov 28, 2024 · 3 comments
Open

originalError undefined in some transaction errors #1716

Angelelz opened this issue Nov 28, 2024 · 3 comments

Comments

@Angelelz
Copy link

I'm currently working through the error log of our service and noticing that some errors during transactions don't have the originalError property and others do:
Example of correct error:

Expected behaviour:

Original error property to exist in the error to verify the underlying problem with the query.
For example, this is an error that originated due to a failed constraint:

{
  "cause": {
    "code": "EREQUEST",
    "originalError": {
      "info": {
        "name": "ERROR",
        "handlerName": "onErrorMessage",
        "number": 2627,
        "state": 1,
        "class": 14,
        "message": "Violation of PRIMARY KEY constraint 'PKSV00301'. Cannot insert duplicate key in object 'dbo.SV00301'. The duplicate key value is (241114-0216      , 1, 0001                     , RAYFLORES  ).",
        "serverName": "SRE-GPSERVER",
        "procName": "",
        "lineNumber": 1
      }
    },
    "name": "RequestError",
    "number": 2627,
    "lineNumber": 1,
    "state": 1,
    "class": 14,
    "serverName": "SRE-GPSERVER",
    "procName": "",
    "precedingErrors": []
  },
  "code": "INTERNAL_SERVER_ERROR",
  "name": "TRPCError",
  "message": "Violation of PRIMARY KEY constr... " // (json truncated)
}

Actual behaviour:

There are other errors in which the originalError property doesn't exists:

{
  "cause": {
    "code": "EABORT",
    "name": "TransactionError"
  },
  "code": "INTERNAL_SERVER_ERROR",
  "name": "TRPCError",
  "message": "Transaction has been aborted.",
  "stack": "TransactionError: Transaction has been aborted.\n    at a._rollback (C:\\Use..." // (stack truncated)
}

Configuration:

const sqlConfig = {
  ...dbCredentials,
  port: 1433,
  requestTimeout: 30000,
  pool: {
    max: env.NODE_ENV === "development" ? 1 : 3,
    min: 0,
    idleTimeoutMillis: 30000,
  },
  options: {
    encrypt: false,
    trustServerCertificate: true,
  },
} satisfies sql.config;

Software versions

  • NodeJS: v21.6.2
  • node-mssql: ^10.0.2
  • SQL Server: Microsoft SQL Server 2019

Why is this error not being instantiated correctly? or is this error originating from within the application?

@Angelelz
Copy link
Author

Angelelz commented Nov 30, 2024

After some debugging and looking into this library's code I realized that rolling back the transaction after an error has been thrown by sql server throws another error. So I had to change my code from this:

    try {
// some code
    } catch (e) {
      await transaction.rollback();
      console.log(e);
      throw e
    }

To this:

    try {
// some code
    } catch (e) {
      try {
        await transaction.rollback();
      } catch (err) {
        console.error("transaction rollback", err);
      }
      console.log(e);
      throw e;
    }

I wasn't aware that rolling back a transaction would throw another error.

Is this intended behavior? If so, it is not totally clear that it is the case by reading the documentation.
Is the request that threw the error still in the queue and that's why the transaction is being marked as aborted?
The documentation states:

If the queue isn't empty, all queued requests will be Cancelled and the transaction will be marked as aborted.

And the fact that it's aborted makes it throw the error?

Also, is there a better pattern for this? Seems to me that having to handle two errors is too much code spaguettization. I'm wondering if I'm just not using the library the way it was intended.

The only reason I have to catch and re-throw is to rollback the transaction.

Edit: I leave it to the authors to close this issue, the only reason I kept it open is to see if I can get some feedback on this particular issue and confirm this is expected behavior.

@dhensby
Copy link
Collaborator

dhensby commented Dec 3, 2024

Thanks for posting the issue and digging into what is going on.

I think what is happening is that xact_abort is on (ie: automatic rollback of transactions on errors) and so you are rolling back a transaction that has already ended (hence the Transaction has been aborted. error).

There is an option that can be set against the tedious driver (abortTransactionOnError) which enables this behaviour, but I see that you don't have that set in your db config. Is there a chance it's enabled at a database level?

@Angelelz
Copy link
Author

Angelelz commented Dec 3, 2024

I do not set abortTransactionOnError in my config and I just just checked in the database:
Screenshot 2024-12-03 at 2 00 38 PM

Also we recently fixed our application hanging due to transactions staying open after an error (never commiting or rolling back). Which is actually why I catch, rollback and re-throw.

Looks like the tedious default for abortTransactionOnError is off according to this issue

I also ruled out an error originating from the application because after making the changes where I catch the Transaction has been aborted. error thrown by the rollback I now see that the error is due to a detected deadlock by sql server.

I guess my next step would be to debug and inspect the state of the transactions and what's causing this._aborted to be set to true at the time of rolling back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants