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

Errors are swallowed non-deterministically #779

Closed
DanielSWolf opened this issue Mar 17, 2022 · 5 comments
Closed

Errors are swallowed non-deterministically #779

DanielSWolf opened this issue Mar 17, 2022 · 5 comments

Comments

@DanielSWolf
Copy link

statement.all() and statement.run() normally forward any SQLite errors by throwing a corresponding SqliteError. Under some circumstances, however, these errors are swallowed instead.

I'd love to give you a compact repro, but I've been trying unsuccessfully for several hours:

  • The problem only occurs on our CI servers (while running unit tests on a large codebase), not locally.
  • It only occurs on Linux machines, not on Windows or macOS.
  • It doesn't occur on every build, but only for about one in two builds without any obvious pattern.
  • If it occurs for a build, it occurs consistently throughout the build: Every single call that should throw doesn't.
  • Everything else works fine: All database mutations and queries work as expected.

I know this is very little to go on. If there is anything else that might help to narrow down the problem, please let me know!

@Prinzhorn
Copy link
Contributor

How did you track this down to better-sqlite3 specifically and not, say, a bug in the test framework? Let me rephrase: are you sure better-sqlite3 does not throw or could the test framework swallow the errors? How does a typical test that fails look like?

The problem only occurs on our CI servers (while running unit tests on a large codebase), not locally.

Your own servers or some cloud provider? What I mean is, is it the same physical hardware (SSD, RAM) or different all the time?

It doesn't occur on every build, but only for about one in two builds without any obvious pattern.

Are these prebuilds or are you compiling better-sqlite3 every time on the CI? In both cases compare the check sums of the node file. If possible pull the entire node_modules from a working CI build vs a failing build and see if something is different.

@DanielSWolf
Copy link
Author

Thanks for getting back to me!

I managed to find out something else:

Normally, when an error occurs, better-sqlite3 throws an object that is-an error. Whenever the problem occurs, an object is thrown, too. It has the correct message, error name, and constructor name, but its parent constructor seems to be wrong. While it is a class called "Error", it is not the class Error. As a result, Jest doesn't treat it as an actual error.

At this point, I'm not quite sure whether the problem lies with better-sqlite3 or with the way Jest manages its processes. 🙁

Regarding your questions:

How did you track this down to better-sqlite3 specifically and not, say, a bug in the test framework?

I added a number of additional tests around the problematic ones to make sure that thrown errors actually get detected.

What I mean is, is it the same physical hardware (SSD, RAM) or different all the time?

They are our own servers. All using the same hardware. And sadly, there is no pattern regarding the build agents.

Are these prebuilds or are you compiling better-sqlite3 every time on the CI? In both cases compare the check sums of the node file.

Will do, but I haven't gotten around to it yet.

@Prinzhorn
Copy link
Contributor

Whenever the problem occurs, an object is thrown, too. It has the correct message, error name, and constructor name, but its parent constructor seems to be wrong. While it is a class called "Error", it is not the class Error. As a result, Jest doesn't treat it as an actual error.

Tests are probably running in a worker? See nodejs/node#37988 and #575
Custom errors don't make it out of the worker.

@Prinzhorn
Copy link
Contributor

Also maybe refs #162 ?

@DanielSWolf
Copy link
Author

Thank you so much for your answers! Now the whole thing makes sense to me. (I've been bitten by Jest's behavior regarding worker threads and serialization in the past, but I didn't connect the dots in this case.)

As a workaround, I now convert all better-sqlite3 errors to plain errors, like this:

try {
  return database.prepare(someQuery).all();
} catch (error) {
  throw new Error(error.message);
}

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

No branches or pull requests

2 participants