pinojs / pino-caller

🌲 Include call site of pino log messages
Other
57 stars 14 forks source link

source-map-support incompatible with tsx #144

Open schnz opened 2 months ago

schnz commented 2 months ago

I am not sure where to open this issue. Ultimately it more likely belongs to either the source-map-support repo or the tsx repo. But I wonder if source-map-support needs to be a requirement at all.

Issue at hand: When using tsx in conjunction with pino and pino-caller, the line numbers in the stack traces are getting messed up.

It is best demonstrated with a minimal example:

Working scenario (without pino-caller)

index.ts

import { pino } from "pino";

const log = pino();

log.error({ err: new Error("artrifical error") }, "error");

Output (correctly reports index.ts:5:18)

$ npx tsx index.ts | npx pino-pretty -S
[12:37:31.996] ERROR (65194): error
    err: {
      "type": "Error",
      "message": "artrifical error",
      "stack":
          Error: artrifical error
              at <anonymous> (/data/user/Downloads/demo/index.ts:5:18)
              at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
              at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
              at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5)
    }

Defect scenario (with pino-caller)

index.ts

import { pino } from "pino";
import pinoCaller from "pino-caller";

const log = pinoCaller.default(pino());

log.error({ err: new Error("artrifical error") }, "error");

Output (incorrectly reports index.ts:1:112)

$ npx tsx index.ts | npx pino-pretty -S
[12:40:21.134] ERROR (66060) <file:///data/user/Downloads/demo/index.ts:1:101>: error
    err: {
      "type": "Error",
      "message": "artrifical error",
      "stack":
          Error: artrifical error
              at file:///data/user/Downloads/demo/index.ts:1:112
              at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
              at ModuleLoader.import (node:internal/modules/esm/loader:316:24)
              at asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5)
    }

This is ultimately caused by the require('source-map-support/register') line in pino-caller. More specifically, it seems to be caused by the Error.prepareStackTrace override that happens inside source-map-support. I didn't dig any deeper than that. But I was wondering whether pino-caller needs this line of code in the first place:

Quote from their README.md:

This package is no longer required as Node 12.12.0 introduced the --enable-source-maps flag.

Node v12 was released 5 years ago and its extended support ran out over 2 years ago. Given all of that, I would argue that removing source-map-support package is an easy fix and it is no longer needed for any sane use-case.

mcollina commented 2 months ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.