Skip to content

fix: replace console.log with structured pino logger in API startup #214

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.

Sign up for GitHub

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

Open
sputnik-mac wants to merge 1 commit into
base: dev
Choose a base branch
from
+7 −4
Open

fix: replace console.log with structured pino logger in API startup #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"multer": "^2.0.1",
"nats": "^2.29.3",
"nestjs-pino": "^4.5.0",
"pino": "^9.6.0",
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's check if the api directory exists and see its structure
ls -la api/ 2>/dev/null | head -20

Repository: alexanderwanyoike/the0

Length of output: 1017

🏁 Script executed:

# Check the current content of api/package.json around line 57
if [ -f "api/package.json" ]; then
  echo "=== api/package.json (lines 50-65) ===" && sed -n '50,65p' api/package.json
else
  echo "api/package.json not found"
fi

Repository: alexanderwanyoike/the0

Length of output: 543

🏁 Script executed:

# Check what lockfiles exist in the api directory
ls -la api/package-lock.json api/yarn.lock 2>&1 | grep -v "cannot access"

Repository: alexanderwanyoike/the0

Length of output: 130

🏁 Script executed:

# Search for CI configuration files and frozen-lockfile references
find . -maxdepth 2 -type f \( -name ".github" -o -name "*.yml" -o -name "*.yaml" \) 2>/dev/null | head -20

Repository: alexanderwanyoike/the0

Length of output: 164

🏁 Script executed:

# Look for CI workflow files
find .github -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null

Repository: alexanderwanyoike/the0

Length of output: 643

🏁 Script executed:

# Check the api.yml workflow file for frozen-lockfile instructions
cat -n .github/workflows/api.yml

Repository: alexanderwanyoike/the0

Length of output: 2694

🏁 Script executed:

# Also check what's actually in the api directory to understand the structure
ls -la api/

Repository: alexanderwanyoike/the0

Length of output: 1017

🏁 Script executed:

# Check if pino is in the yarn.lock file
grep -n "pino" api/yarn.lock | head -20

Repository: alexanderwanyoike/the0

Length of output: 1478

🏁 Script executed:

# Let's also check the full dependency section in package.json to understand the context
head -70 api/package.json | tail -25

Repository: alexanderwanyoike/the0

Length of output: 740

🏁 Script executed:

# Check what version of pino-http requires and whether it conflicts with pino ^9.6.0
grep -A5 "pino-http@\^11.0.0:" api/yarn.lock

Repository: alexanderwanyoike/the0

Length of output: 372

🏁 Script executed:

# Check if there are any other pino entries in the lockfile that could satisfy ^9.6.0
grep "^pino@" api/yarn.lock

Repository: alexanderwanyoike/the0

Length of output: 79

🏁 Script executed:

# Let's get more context on the pino-http dependency and what it requires
grep -B2 -A10 "pino-http@\^11.0.0:" api/yarn.lock

Repository: alexanderwanyoike/the0

Length of output: 497

🏁 Script executed:

# Check if pino-http requires pino ^10.0.0 specifically
grep -A20 "pino-http@\^11.0.0:" api/yarn.lock | grep -A5 "dependencies:"

Repository: alexanderwanyoike/the0

Length of output: 336

🏁 Script executed:

# Let's see if pino ^9.6.0 exists in the lockfile at all
grep "^pino@\^9" api/yarn.lock

Repository: alexanderwanyoike/the0

Length of output: 48

Update the pino version to satisfy existing dependency constraints.

Line 57 adds "pino": "^9.6.0", but pino-http@^11.0.0 (already in dependencies) requires pino@^10.0.0. This version conflict will cause yarn install --frozen-lockfile to fail in CI. Either update pino to ^10.0.0 or downgrade pino-http to a version compatible with pino ^9.6.0, then run yarn install in api/ and commit the updated lockfile.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/package.json` at line 57, Change the conflicting dependency so
pino-http's peer requirement is satisfied: update the "pino" dependency in
package.json from "^9.6.0" to "^10.0.0" (or alternatively downgrade "pino-http"
to a version compatible with pino "^9.6.0"), then run yarn install in the api/
directory and commit the resulting lockfile so CI's yarn install
--frozen-lockfile will succeed; target the "pino" and "pino-http" entries when
making the change.

"passport": "^0.6.0",
"passport-jwt": "^4.0.1",
"passport-local": "^1.0.0",
Expand Down
10 changes: 6 additions & 4 deletions api/src/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { ValidationPipe } from "@nestjs/common";
import { NestFactory } from "@nestjs/core";
import { Logger } from "nestjs-pino";
import pino from "pino";
import { AppModule } from "./app.module";
import { runMigrations } from "./database/migrate";

const bootstrapLogger = pino({ name: "bootstrap" });

async function bootstrap() {
// Run database migrations before starting the application
// Note: migrations run before NestJS app is created, so we use console here
console.log("Running database migrations...");
bootstrapLogger.info("Running database migrations...");
await runMigrations();
console.log("Database migrations completed");
bootstrapLogger.info("Database migrations completed");

const app = await NestFactory.create(AppModule, { bufferLogs: true });

Expand Down Expand Up @@ -39,6 +41,6 @@ async function bootstrap() {
}

bootstrap().catch((error) => {
console.error("Failed to start the0 API:", error);
bootstrapLogger.error({ err: error }, "Failed to start the0 API");
process.exit(1);
Comment on lines +44 to 45
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and examine lines around 44-45
if [ -f "api/src/main.ts" ]; then
  echo "=== File: api/src/main.ts ===" 
  wc -l api/src/main.ts
  echo "=== Lines 40-50 context ==="
  sed -n '40,50p' api/src/main.ts | cat -n
else
  echo "File api/src/main.ts not found"
  git ls-files | grep "main.ts"
fi

Repository: alexanderwanyoike/the0

Length of output: 367

🏁 Script executed:

# Search for bootstrap function and error handling patterns
rg "bootstrap.*catch|process\.exit" api/src/ -A 3 -B 3

Repository: alexanderwanyoike/the0

Length of output: 2067

🏁 Script executed:

# Check how the logger is initialized to understand pino setup
rg "bootstrapLogger|pino|winston" api/src/main.ts -B 2 -A 2

Repository: alexanderwanyoike/the0

Length of output: 801

🏁 Script executed:

# Check pino configuration and understand default behavior
rg "pino|transport|stream" api/src/main.ts api/src/app.module.ts -B 2 -A 2

Repository: alexanderwanyoike/the0

Length of output: 559

🏁 Script executed:

# Check if there are any pino transport configurations or logging setup
fd -e ts -e js api/src | xargs grep -l "pino\|Logger" | head -10

Repository: alexanderwanyoike/the0

Length of output: 399

🌐 Web query:

pino logger process.exit synchronous stdout flushing Node.js best practices

💡 Result:

In Node.js, when using Pino logger and calling process.exit, logs may not flush to stdout synchronously by default, potentially losing recent entries due to buffering in sonic-boom (Pino's underlying writer). This is a known issue discussed extensively in Pino GitHub issues since v5+. Best practices for synchronous stdout flushing before process.exit: 1. Use pino.destination explicitly (sonic-boom defaults to sync: true for stdout/files): import pino from 'pino'; const dest = pino.destination; // or pino.destination(1) for explicit stdout fd const logger = pino(dest); logger.info('Will flush'); process.exit(1); // Logs guaranteed due to synchronous writes This ensures writes are synchronous (fs.writeSync), preventing loss on abrupt exit. Performance trade-off: slower than async but safe for exit scenarios. 2. For async destinations (sync: false), call logger.flushSync or dest.flushSync before exit: const dest = pino.destination({ sync: false }); const logger = pino(dest); logger.info('Flush me'); dest.flushSync; // Synchronous buffer flush process.exit(1); Note: logger.flush is async (fire-and-forget); use flushSync for blocking guarantee before exit. 3. Use logger.fatal: Automatically sync-flushes as it's designed for pre-exit logging. logger.fatal('Exiting with error'); process.exit(1); // Flushed automatically 4. Avoid transports like pino-pretty in production/exit-critical paths: They run in worker threads, causing flush races (Issue #1705). Use { sync: true } if needed, but prefer raw JSON for stdout. const stream = pino.transport({ target: 'pino-pretty', options: { sync: true } }); const logger = pino(stream); // Still risky on exit; test thoroughly 5. Graceful shutdown handlers (preferred for signals, not direct process.exit): import onExit from 'on-exit-leak-free'; // Used internally by Pino onExit( => { logger.flushSync; process.exit(0); }); Pino registers these automatically in modern Node (v14+), but direct process.exit bypasses 'beforeExit'. Direct process.exit terminates immediately after synchronous code, so synchronous writes/flushes are essential. For high-perf async logging, periodic logger.flush (e.g., every 10s) reduces risk, but sync destination is simplest for exit safety. Test: Modern Pino (v8+) with pino.destination reliably flushes on process.exit(1).

Citations:

Ensure error logs flush before exiting during bootstrap failures.

Line 46 calls process.exit(1) immediately after logging; this can truncate the fatal error log because process.exit() bypasses Node's graceful shutdown and stream flushing. Use process.exitCode = 1 with return to allow pino to flush logs to stdout before termination.

Proposed change 
bootstrap().catch((error) => {
  bootstrapLogger.error({ err: error }, "Failed to start the0 API");
-  process.exit(1);
+  process.exitCode = 1;
+  return;
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bootstrapLogger.error({ err: error }, "Failed to start the0 API");
process.exit(1);
bootstrap().catch((error) => {
bootstrapLogger.error({ err: error }, "Failed to start the0 API");
process.exitCode = 1;
return;
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@api/src/main.ts` around lines 44 - 45, Replace the immediate process.exit(1)
call after bootstrapLogger.error to allow pino to flush: set process.exitCode =
1 and return from the bootstrap/startup flow instead of calling process.exit(1),
so leave the bootstrapLogger.error(...) call intact but replace the terminating
call (process.exit(1)) with assigning process.exitCode = 1 and returning from
the function that runs startup (where main/bootstrap logic executes) to let logs
be flushed.

});
Loading