Closed laurenzhonauer closed 1 year ago
Don't use NODE_ENV, it's an antipattern that results in production issues and it makes it harder to debug things. We do not recommend that for this reason. The best way to handle this is to use dotenv and have PINO_PRETTY env to trigger this on and off.
Yes I agree with you. But I think this is not mentioned in the documentation.
So if you create blank node program, add pino and follow the programmatic integration documentation for pino-pretty, you will get the error automatically. With blank programm i mean, no extra dependencies, so no dotenv (I assume you mean this).
Also I would argue that the majority of people are not using dotenv and are relying on NODE_ENV - antipattern or not. (For example Next.js relies on NODE_ENV out of the box)
Anyway, could you provide a piece of example code that connects the dots between pino-pretty and dotenv. That would be very helpful :)
Unfortuantely I do not have much time. Some piece of docs around this would be great, feel free to send a PR but do not mention NODE_ENV.
Fair enough, in that case even bigger thanks for taking the time to look over the issue.
I will try to find some time to create a pr for this.
In case anyone stumbles upon this, before a pr is open - Feel free ^^
Thanks @laurenzhonauer for the info - if I had not stumbled upon this probably wouldn't have been able to fix my own issue
just for anyone reading:
import pino from "pino";
import { env } from "@/env.mjs";
const levelFormatter = (_: unknown, number: number): { severity: string } => {
let severity;
switch (number) {
case 10:
case 20: {
severity = "DEBUG";
break;
}
case 30: {
severity = "INFO";
break;
}
case 40: {
severity = "WARNING";
break;
}
case 50: {
severity = "ERROR";
break;
}
case 60: {
severity = "CRITICAL";
break;
}
default: {
severity = "DEFAULT";
break;
}
}
return { severity };
};
const timestampFunction = (): string =>
`,"timestamp":"${new Date().toISOString()}"`;
const prodOptions: pino.LoggerOptions = {
base: undefined,
formatters: {
level: levelFormatter,
},
messageKey: "message",
timestamp: timestampFunction,
};
const getDevStream = (): pino.DestinationStream => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-var-requires
const pretty = require("pino-pretty");
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
return pretty({
levelFirst: true,
colorize: true,
ignore: "time,hostname,pid",
}) as pino.DestinationStream;
};
export const logger =
env.NODE_ENV === "production"
? pino(prodOptions)
: pino({ level: "debug" }, getDevStream());
Description
In the Readme under Programmatic Integration it is stated that pino-pretty should be added as dev-dependency and then be configured as following:
If one would follow those steps exactly, they would get following error in Production.
Steps to reproduce
export NODE_ENV=production
)npm ci
Expected Result
Pino should print un-prettified messages
Actual Result
Error is thrown due to missing pino-pretty (Because it is only a dev dependency)
Proposal
I guess the Documentation just needs to say that the transport option should only be added after a NODE_ENV check. Maybe its enough to just add that to the Readme. If you accept that i can create a pr myself :)