lz4 check fails in jest

[gabegorelick]

The optional lz4 dependency is currently loaded as follows:

function tryLoadLZ4Module(): LZ4Module | undefined {
  try {
    return require('lz4'); // eslint-disable-line global-require
  } catch (err) {
    const isModuleNotFoundError = err instanceof Error && 'code' in err && err.code === 'MODULE_NOT_FOUND';
    if (!isModuleNotFoundError) {
      throw err;
    }
  }
}

See https://github.com/databricks/databricks-sql-nodejs/blob/56bd0d90d61a2bdff4e62cd55ebcb8d070fa60e2/lib/utils/lz4.ts.

However, this fails when running under Jest due to https://github.com/jestjs/jest/issues/2549 (see also https://github.com/jestjs/jest/issues/11808). Namely, the err instanceof Error check returns false since Jest substitutes its own objects for JS globals. This causes calling code to fail with a ModuleNotFound error.

Is the err instanecof Error check really needed here? Can it be removed?

[kravets-levko]

@gabegorelick Yes, this check is needed because otherwise err has unknown type and TS requires a type assertion before it can be used. Also, there's nothing wrong with this code. The problem comes from Jest messing up globals, and it has to be fixed in Jest.

[haggholm]

@gabegorelick Yes, this check is needed because otherwise err has unknown type and TS requires a type assertion before it can be used.

That doesn't mean it has to be an instanceof check, though. E.g. maybe something like err && typeof err === "object" && "code" in err …

Also, there's nothing wrong with this code. The problem comes from Jest messing up globals, and it has to be fixed in Jest.

I don't really disagree with this; fixing the issue by modifying your library would definitely be going out of your way to make it work with the quirks of certain widely-used tools. But…maybe that's not such a terrible thing? After all it's Jest, not some random test framework written by one guy and used in-house by one small company. It is a workaround for one particular framework, but it's one of the most widely-used, so it's a workaround that in the longer term is likely to help a lot of your users.

[gabegorelick]

That doesn't mean it has to be an instanceof check, though

I would also suggest that something like err as Error may be more appropriate than an instanecof check if the check is truly just to placate Typescript.

[DarkPhoenix2704]

node:internal/modules/cjs/loader:1734
  return process.dlopen(module, path.toNamespacedPath(filename));
                 ^

Error: Module did not self-register: '/Users/anbarasun/WebstormProjects/nocohub/node_modules/.pnpm/[email protected]/node_modules/lz4/build/Release/xxhash.node'.
    at Object..node (node:internal/modules/cjs/loader:1734:18)
    at Module.load (node:internal/modules/cjs/loader:1318:32)
    at Function._load (node:internal/modules/cjs/loader:1128:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:219:24)
    at Module.require (node:internal/modules/cjs/loader:1340:12)
    at require (node:internal/modules/helpers:138:16)
    at Object.<anonymous> (/Users/anbarasun/WebstormProjects/nocohub/node_modules/.pnpm/[email protected]/node_modules/lz4/lib/utils.js:4:11)
    at Module._compile (node:internal/modules/cjs/loader:1565:14)
    at Object..js (node:internal/modules/cjs/loader:1708:10) {
  code: 'ERR_DLOPEN_FAILED'
}

I get this error, when using in Node 22. This is a blocker for us to migrate to Node 22

[Malal884]

I also have problem with lz4 using webpack. Get the following error: WARNING in ./node_modules/@databricks/sql/dist/utils/lz4.js 5:15-29 Module not found: Error: Can't resolve 'lz4' in 'C:\workspaces\temp\udm-service\node_modules@databricks\sql\dist\utils' Did you mean './lz4'? Requests that should resolve in the current directory need to start with './'. Requests that start with a name are treated as module requests and resolve within module directories (node_modules). If changing the source code is not an option there is also a resolve options called 'preferRelative' which tries to resolve these kind of requests in the current directory too. @ ./node_modules/@databricks/sql/dist/utils/index.js 37:30-46 @ ./node_modules/@databricks/sql/dist/index.js 23:16-34

[jansepke]

we are also having this issue but with all node versions and independent of jest https://github.com/databricks/databricks-sql-nodejs/issues/289