fix(caller): use calling file name for libraries calling Pino

[FredericEspiau]

Excluding all the files in node_modules didn't work with nestjs-pino

I guess because there are too many files before reaching the user's files

This will return the file in second position in the stacktrace as get-caller-file did if we can't find a file outside node_modules

Stacktrace:

/Users/my-repo/node_modules/one-dependency/packages/file1.js
/Users/my-repo/node_modules/one-dependency/packages/file2.hs
/Users/my-repo/node_modules/pino-http/logger.js
/Users/my-repo/node_modules/pino-http/logger.js
/Users/my-repo/node_modules/nestjs-pino/LoggerModule.js
/Users/my-repo/node_modules/nestjs-pino/LoggerModule.js
/Users/my-repo/node_modules/@nestjs/core/middleware/middleware-module.js
/Users/my-repo/node_modules/@nestjs/core/middleware/middleware-module.js

Other example:

/Users/my-repo/node_modules/nestjs-pino/PinoLogger.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js
/Users/my-repo/node_modules/@nestjs/core/injector/injector.js

I wasn't able to create a test for this case

This fixes #1277

[mcollina]

Could you add a test?

[FredericEspiau]

I will try, I just had an idea on how to do it

[mkaufmaner]

Oh this seems to be related; #1278

[FredericEspiau]

Yup its the same issue but I won't update my PR before tomorrow (2021-12-21T10:00:00+01:00)

[mcollina]

Don't use the ? as it's not in node v12.

[jsumners]

Also, a comment explaining what this is would be very helpful.

[FredericEspiau]

done, this also handles #1278

[mcollina]

lgtm

[mcollina]

Thanks!

[mcollina]

@ShogunPanda could you take a look?

[ShogunPanda]

Single nit, but in general LGTM!

[ShogunPanda]

Little nit: I would join isFileName and isOutsideNodeModules as follow to avoid extra context switch:

function isOutsideNodeModules (file) {
  return typeof input === 'string' && fs.existsSync(input) && file.indexOf('node_modules') === -1
}}

[FredericEspiau]

done

[mcollina]

CI is failing.

[FredericEspiau]

I can't reproduce the issue on my computer, I don't know what I can do to prevent the CI from failing 🤔

[ShogunPanda]

@FredericEspiau I found the problem: you have several files (like fixtures/eval/node_modules/2-files.js) which are not included in the commit since node_modules is in .gitignore.

You have to explicitly add to the PR or, even better, provide them as an archive which you can unarchive during test time in a temporary folder.

[FredericEspiau]

Excellent finding, indeed while doing my dev I kept on thinking that I should remove the this folder from the gitignore and then I forgot! Thanks I'll do it asap

[FredericEspiau]

Awesome, the CI passed !

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.