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

Sequelize errors don't print properly in jest #14807

Open
3 of 6 tasks
benasher44 opened this issue Jul 26, 2022 · 28 comments · Fixed by #15559
Open
3 of 6 tasks

Sequelize errors don't print properly in jest #14807

benasher44 opened this issue Jul 26, 2022 · 28 comments · Fixed by #15559
Labels
existing workaround For issues. There is a known workaround for this issue. funded type: bug

Comments

@benasher44
Copy link

benasher44 commented Jul 26, 2022

Issue Creation Checklist

  • I understand that my issue will be automatically closed if I don't fill in the requested information
  • I have read the contribution guidelines

Bug Description

When using sequelize in jest, errors don't print their message, which makes debugging difficult. You get a trace back into the query interface in sequelize, but you have to set breakpoints in sequelize in order to determine the nature of the error (syntax error, foreign key constraint issue, etc.).

I have verified that other thrown errors print properly, so it seems to be an issue with these in particular in Sequelize.

Reproducible Example

it("passes", async () => {
  await db.query("SELECT 1 )").
});

In this example, let's assume I want to write a test that selects 1 and passes. Assuming db is a Sequelize instance pointing at a postgres db, this test that we want to be successful fails due to a syntax error. In the console, I get a trace, but the error message is missing.

What do you expect to happen?

The error message prints indicating the sql syntax error. The full printout should look someting like:

DatabaseError: syntax error at or near ")"

      at Query.run (../node_modules/sequelize/src/dialects/postgres/query.js:76:25)
      at ../node_modules/sequelize/src/sequelize.js:641:28
      at PostgresQueryInterface.select (../node_modules/sequelize/src/dialects/abstract/query-interface.js:1001:12)
      at … remaining trace back to test

What is actually happening?

The error is printed, but only the stack trace appears, which will look something like this (note that message is omitted):

      at Query.run (../node_modules/sequelize/src/dialects/postgres/query.js:76:25)
      at ../node_modules/sequelize/src/sequelize.js:641:28
      at PostgresQueryInterface.select (../node_modules/sequelize/src/dialects/abstract/query-interface.js:1001:12)
      at … remaining trace back to test

Environment

  • Sequelize version: 6.21.3
  • Node.js version: 16.15.0
  • If TypeScript related: TypeScript version:
  • Database & Version: Postgres 11.x
  • Connector library & Version: pg@8.7.3

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I will need guidance.
  • No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • No, I don't have the time, and I understand that I will need to wait until someone from the community or maintainers is interested in resolving my issue.

Indicate your interest in the resolution of this issue by adding the 👍 reaction. Comments such as "+1" will be removed.

@WikiRik
Copy link
Member

WikiRik commented Aug 2, 2022

This is not very developer friendly. Thanks for bringing this to our attention, we'll take a look at this soon

@benasher44
Copy link
Author

No problem. Thank you!

@WikiRik
Copy link
Member

WikiRik commented Aug 12, 2022

I've started to look into this and you can use the parent or original property in v6 to retrieve the message you need (together with additional information). In v7 we are using the native Error#cause (or the cause property in versions before Node 16.19).

By the way, this issue is similar to #13978

@benasher44
Copy link
Author

Ah nice! Seems like we can close this out then, since it's fixed in v7?

@WikiRik
Copy link
Member

WikiRik commented Aug 13, 2022

Well, kinda? The basic printout seems to be the same in v7 (using Node 18), it's just that we updated the way that additional information is displayed. I'll keep this open for now but by logging the resulting error you can find enough information so breakpoints should be less necessary.

@WikiRik WikiRik added the existing workaround For issues. There is a known workaround for this issue. label Aug 13, 2022
@benasher44
Copy link
Author

Ah okay. How do you get the resulting error in jest? I've been digging around, but I haven't figured out where in Jest I can add hooks to be able to do this. Otherwise, it seems that the best way is to just wrap the db call in a try catch and access it that way.

@tomquist
Copy link

I was digging a bit deeper into this and found that jest reads the message from the error's stack property. However, since Sequelize injects the stack trace from a dummy error generated just for getting a stack trace the message is lost, e.g. see here:

const errForStack = new Error();

@WikiRik
Copy link
Member

WikiRik commented Aug 19, 2022

Ah yes, this was already mentioned in #14408
This was introduced in #13347 and I think it's a good change but we just need to see if we can return the original error (because of the name) with the injected stack trace.

@tomquist
Copy link

We're working around this now using this function:

function monkeyPatchSequelizeErrorsForJest(instance: Sequelize) {
  if (typeof jest === "undefined") {
    return;
  }
  const origQueryFunc = instance.query;
  instance.query = function (this: Sequelize, ...args: any[]) {
    return origQueryFunc.apply(this, args as any).catch(async (err) => {
      if (typeof err.stack === "string") {
        const stackLines = err.stack.split("\n");
        if (stackLines[0] === "Error: ") {
          err.stack = `${stackLines[0]}${err.message}\n${stackLines.slice(1).join("\n")}`;
        }
      }
      throw err;
    }) as any; // Cast to any to hide that it's a promise. Otherwise jest will show this code as the root cause of the error
  } as typeof origQueryFunc;
}

Couldn't the stack-trace be captured using this function and pass in the method to exclude (formatError):
https://nodejs.org/api/errors.html#errorcapturestacktracetargetobject-constructoropt

@WikiRik WikiRik self-assigned this Sep 15, 2022
@WikiRik WikiRik removed their assignment Oct 24, 2022
@WikiRik WikiRik added the funded label Nov 5, 2022
@rolandm2017
Copy link

rolandm2017 commented Nov 11, 2022

Any word on this being fixed? I read there's a workaround: how do I use that block of code posted above this msg to see a Sequelize error? It's blocking me from making my unit tests work, in about 20 different places...

e: I might be willing to help fix this, since I have no other path forward, but it looks complicated and beyond my abilities to fix within, say, two weeks

edit2: 14 days later I still have trouble reading sequelize error messages in jest. This edit is here to let the team know someone cares about this.

@WikiRik
Copy link
Member

WikiRik commented Nov 28, 2022

@fzn0x you mentioned you wanted to look into this;
@Plutownium found this StackOverflow question that might help; https://stackoverflow.com/questions/62897235/how-to-see-stacktrace-cause-of-an-error-in-jest The fix mentioned there was merged in Node 16 (and 18), but not Node 14. So do check how the expectation is for both versions. And be cautious since I'm not sure if this is the same issue as this here.

Basically what needs to happen is that we need the message of the original error with the stack trace of the dummy error (see #14807 (comment) and #14807 (comment) )

@erfanium
Copy link

erfanium commented Nov 28, 2022

I believe this change can fix this issue.
Change this:

throw this.formatError(error, errForStack.stack);

To something like this:

 throw this.formatError(error, isJest ? error.stack : errForStack.stack); 

for all dialects

@ephys
Copy link
Member

ephys commented Nov 28, 2022

We should fix this issue in all environments, it's a problem in mocha and other tools that only print the .stack property

Following #13347, the stack went from

SequelizeDatabaseError: Invalid column name 'unknown_column'.
    at Query.formatError (/Users/harry/code/sequelize/lib/dialects/mssql/query.js:314:12)
    at Query._run (/Users/harry/code/sequelize/lib/dialects/mssql/query.js:93:18)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

To

Error
    at Query.run (/Users/harry/code/sequelize/lib/dialects/mssql/query.js:119:25)
    at /Users/harry/code/sequelize/lib/sequelize.js:619:28
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async Context.<anonymous> (/Users/harry/code/sequelize/test/integration/sequelize/query.test.js:319:11)

But it should have become this instead:

SequelizeDatabaseError: Invalid column name 'unknown_column'.
    at Query.run (/Users/harry/code/sequelize/lib/dialects/mssql/query.js:119:25)
    at /Users/harry/code/sequelize/lib/sequelize.js:619:28
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async Context.<anonymous> (/Users/harry/code/sequelize/test/integration/sequelize/query.test.js:319:11)

It should merge both stacks (keeping the error message but replacing the trace) instead of fully replacing it.

@fzn0x
Copy link
Member

fzn0x commented Nov 28, 2022

@fzn0x you mentioned you wanted to look into this; @Plutownium found this StackOverflow question that might help; https://stackoverflow.com/questions/62897235/how-to-see-stacktrace-cause-of-an-error-in-jest The fix mentioned there was merged in Node 16 (and 18), but not Node 14. So do check how the expectation is for both versions. And be cautious since I'm not sure if this is the same issue as this here.

Basically what needs to happen is that we need the message of the original error with the stack trace of the dummy error (see #14807 (comment) and #14807 (comment) )

Thanks, I think the expectation still to show the message with the improved stack trace

@rolandm2017
Copy link

rolandm2017 commented Nov 28, 2022

For anyone else who is stuck because they can't see the full error in Jest: A buddy told me "try using the debugger" and that worked. I spent 2 hrs fixing the bug but 50% of that was from learning the debugger. A big improvement over "stuck, permanently stuck".

@rolandm2017
Copy link

rolandm2017 commented Dec 24, 2022

What are other programmers doing to debug errors while Jest isn't reporting them correctly? My portfolio project is held up due to this bug ... need my portfolio project done fast

edit: I discovered that I could get the truncated error message to print in full by surrounding my Sequelize code with

    public getAllBatchNums = async () => {
        try {
            const batches = await Batch.findAll({});
            const justNums: number[] = batches.map(b => b.batchId);
            return justNums;
        } catch (err) {
            console.log(err);
            throw err;
        }
    };

Had to do it in like 40 places to find the right one but, I got it! Hope this helps someone

@javiermrz
Copy link

javiermrz commented Jan 7, 2023

Any news?? This is happening to me with NestJS and Sequelize. (not with Jest, Mocha, etc.)

@javiermrz
Copy link

javiermrz commented Jan 7, 2023

Surrouding the code with a try/catch and turning the exception to a string seems to do the trick until this gets fixed!!

 try {
    await doDbStuffThatFails();
 } catch (e) {
    console.log(e.toString());
 }

@ephys
Copy link
Member

ephys commented Jan 15, 2023

Reopening until the fix has been back ported to v6

@ephys ephys reopened this Jan 15, 2023
@aronwoost
Copy link

Looking forward for the back port. Will improve dev experience. 👍

@adamhowardprice
Copy link

Just checking in again to ask about the status of backporting this change to v6. Thanks!

@ak-beam
Copy link

ak-beam commented Dec 13, 2023

Is there a good workaround for v6 at the moment? This is causing our team a lot of difficulty when we have Sequelize errors in CI.

@Naktibalda
Copy link

@ak-beam catching error and logging it using console.log is a simple but noisy workaround.

I created this error wrapper which shows correct error message, SQL statement, parameters and displays correct trace:

class ImprovedSequelizeError extends Error {
  constructor(originalError: SequelizeError) {
    super();
    this.name = originalError.name;

    let { message } = originalError.parent;
    if (originalError.sql) {
      message += "\nSQL: " + originalError.sql;
    }

    if (originalError.parameters) {
      const stringifiedParameters = JSON.stringify(originalError.parameters);
      if (
        stringifiedParameters !== "undefined" &&
        stringifiedParameters !== "{}"
      ) {
        message += "\nParameters: " + stringifiedParameters;
      }
    }

    message += "\n" + originalError.stack;

    this.message = message;

    Error.captureStackTrace(this, fixSequelizeError);
  }
}

const isSequelizeError = (e: unknown): e is SequelizeError =>
  e instanceof Error && e.name.startsWith("Sequelize");

const fixSequelizeError = (e: unknown) => {
  if (isSequelizeError(e)) {
    throw new ImprovedSequelizeError(e);
  }

  throw e;
};

We use fixSequelizeError instead of console.log in the tests.

try {
  // test code
} catch (e) {
  fixSequelizeError(e);
}

@ak-beam
Copy link

ak-beam commented Dec 13, 2023

Based on a comment above, this works to at least print the error (although Jest still prints the truncated version later).

Advantage of this is you can put it in the source code where the Sequelize instance is defined rather than changing all your tests.

function monkeyPatchSequelizeErrorsForJest(instance: Sequelize) {
  if (typeof jest === 'undefined') {
    return instance
  }
  const origQueryFunc = instance.query
  instance.query = async function query(this: Sequelize, ...args: any[]) {
    let result
    try {
      result = await origQueryFunc.apply(this, args as any)
    } catch (err: any) {
      console.error(err) // Important - this is how we debug the error
      throw err
    }
    return result
  } as typeof origQueryFunc
  return instance
}

export const db = monkeyPatchSequelizeErrorsForJest(new Sequelize(databaseUrl))

@klondikemarlen
Copy link

For added safety I tweaked this a bit to run

// api/src/db/utils/monkey-patch-sequelize-errors-for-jest.ts
import { Sequelize } from "sequelize"

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const sequelizeVersion = (Sequelize as any).version
const major = sequelizeVersion.split(".").map(Number)[0]

if (major >= 7) {
  console.warn("This patch was probably made redundant in Sequelize v7, you should check!")
}

/**
 * Fixed in Sequelize v7, but hasn't been back-ported to Sequelize v6.
 * See https://github.com/sequelize/sequelize/issues/14807#issuecomment-1854398131
 */
export function monkeyPatchSequelizeErrorsForJest(instance: Sequelize) {
  if (typeof jest === "undefined") return instance

  const origQueryFunc = instance.query
  instance.query = async function query(this: Sequelize, ...args: any[]) {
    let result
    try {
      result = await origQueryFunc.apply(this, args as any)
    } catch (err: any) {
      console.error(err) // Important - this is how we debug the error
      throw err
    }
    return result
  } as typeof origQueryFunc

  return instance
}

Usage

// api/src/db/db-client.ts
import { Sequelize, Options } from "sequelize"
import { monkeyPatchSequelizeErrorsForJest } from "@/db/utils/monkey-patch-sequelize-errors-for-jest"

export const SEQUELIZE_CONFIG: Options = {
  // ... your config
}

let db: Sequelize
if (NODE_ENV === "test") {
  db = monkeyPatchSequelizeErrorsForJest(new Sequelize(SEQUELIZE_CONFIG))
} else {
  db = new Sequelize(SEQUELIZE_CONFIG)
}

export default db

klondikemarlen added a commit to icefoganalytics/internal-data-portal that referenced this issue Feb 6, 2024
@mltsy
Copy link

mltsy commented Apr 11, 2024

Thanks for the workaround solutions! I combined the ImprovedSequelizeError class from @Naktibalda and the sequelize money-patcher from @ak-beam and @klondikemarlen to create a monkey-patch that actually just throws a (sorta) correct error instance: https://gist.github.com/mltsy/629d2b5b703f14e3e97eb8396518cfad

(The stack trace has a few more frames than it normally should, but it's usable!)

@0tii
Copy link

0tii commented Apr 15, 2024

I cant believe this is a problem since 2022 and it is still not fixed! I am encountering it too in my nestjs application, not even in jest, receiving absolutely nothing-saying errors.

Worse even: In nestjs the sequelize instantiation is abstracted away by the SequelizeModule so applying these monkey patches isnt even an option.

@ephys
Copy link
Member

ephys commented Apr 16, 2024

It was fixed for Sequelize 7 in #15559

You can still access the Sequelize instance, even with SequelizeModule, as you can inject it

Writing your own provider is also pretty straightforward, especially in v7:

import type { Provider } from '@nestjs/common';
import { Sequelize, importModels } from '@sequelize/core';
import { PostgresDialect } from '@sequelize/postgres';

export const sequelizeProvider: Provider = {
  provide: Sequelize,
  inject: [ConfigService],
  useFactory: async (configService: ConfigService): Promise<Sequelize> => {
    const sequelize = new Sequelize({
      dialect: PostgresDialect,
      url: configService.get('databaseUrl'),
      models: await importModels(`${import.meta.dirname}/**/*.model.{ts,js}`),
    });

    // wait for the database to be up and running
    await sequelize.authenticate({
      retry: {
        backoffBase: 5000,
        backoffExponent: 1.05,
        max: Infinity,
        timeout: 5000,
      },
    });

    return sequelize;
  },
};

klondikemarlen added a commit to icefoganalytics/travel-authorization that referenced this issue Apr 18, 2024
klondikemarlen added a commit to icefoganalytics/travel-authorization that referenced this issue Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
existing workaround For issues. There is a known workaround for this issue. funded type: bug
Projects
Status: v6