fix: guard pino-pretty for serverless environments#1523
fix: guard pino-pretty for serverless environments#1523Omramanuj wants to merge 1 commit intobrowserbase:mainfrom
Conversation
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/core/package.json">
<violation number="1" location="packages/core/package.json:110">
P3: Missing trailing newline at end of package.json will cause prettier --check to fail</violation>
</file>
<file name="packages/server/src/server.ts">
<violation number="1" location="packages/server/src/server.ts:56">
P2: ESM module uses undefined `require` to detect `pino-pretty`, causing silent fallback and loss of pretty logs</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }), | ||
| ...(process.env.NODE_ENV === "development" && (() => { | ||
| try { | ||
| require.resolve("pino-pretty"); |
There was a problem hiding this comment.
P2: ESM module uses undefined require to detect pino-pretty, causing silent fallback and loss of pretty logs
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/server/src/server.ts, line 56:
<comment>ESM module uses undefined `require` to detect `pino-pretty`, causing silent fallback and loss of pretty logs</comment>
<file context>
@@ -51,15 +51,23 @@ const app = fastify({
- }),
+ ...(process.env.NODE_ENV === "development" && (() => {
+ try {
+ require.resolve("pino-pretty");
+ return {
+ transport: {
</file context>
| }, | ||
| "homepage": "https://stagehand.dev" | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
P3: Missing trailing newline at end of package.json will cause prettier --check to fail
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/package.json, line 110:
<comment>Missing trailing newline at end of package.json will cause prettier --check to fail</comment>
<file context>
@@ -107,4 +107,4 @@
},
"homepage": "https://stagehand.dev"
-}
+}
\ No newline at end of file
</file context>
| } | |
| } | |
Greptile OverviewGreptile SummaryThis PR fixes crashes in serverless environments by making Changes MadeCore Package (
Server Package (
How It WorksWhen initializing loggers:
This ensures Stagehand works correctly in serverless environments like Vercel where optional dependencies are excluded from deployment bundles. Implementation QualityThe implementation correctly addresses the issue with proper error handling and fallback behavior. The lockfile changes show that pnpm correctly marked pino-pretty and its transitive dependencies as optional. Minor style improvements suggested: trailing whitespace cleanup and consideration of ES module-standard approaches for module resolution in the server package. Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant App as Application
participant Logger as Logger/Fastify
participant Node as Node.js Runtime
participant PinoPretty as pino-pretty (optional)
App->>Logger: Initialize logger with pretty option
alt In Development/Pretty Mode
Logger->>Node: require.resolve("pino-pretty")
alt pino-pretty available
Node-->>Logger: Module path found
Logger->>PinoPretty: Configure pino transport
PinoPretty-->>Logger: Pretty formatter ready
Logger-->>App: Logger with pretty printing
else pino-pretty not available
Node-->>Logger: MODULE_NOT_FOUND error
Logger->>Logger: Fallback to standard logging
Logger-->>App: Logger with standard output
end
else Production/No Pretty Mode
Logger-->>App: Logger with standard output
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "zod": "^4.2.0" | ||
| }, | ||
| "optionalDependencies": { | ||
| "pino-pretty": "^11.3.0" |
There was a problem hiding this comment.
Trailing whitespace after "^11.3.0" should be removed
| "pino-pretty": "^11.3.0" | |
| "pino-pretty": "^11.3.0" |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/server/package.json
Line: 36:36
Comment:
Trailing whitespace after `"^11.3.0"` should be removed
```suggestion
"pino-pretty": "^11.3.0"
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| ...(process.env.NODE_ENV === "development" && (() => { | ||
| try { | ||
| require.resolve("pino-pretty"); | ||
| return { | ||
| transport: { | ||
| options: { | ||
| colorize: true, | ||
| ignore: "pid,hostname", | ||
| }, | ||
| target: "pino-pretty", | ||
| }, | ||
| }; | ||
| } catch { | ||
| // pino-pretty not available, use standard logging | ||
| return {}; | ||
| } | ||
| })()), |
There was a problem hiding this comment.
Using require.resolve() in an ES module context (this package has "type": "module" in package.json) is non-standard. While it may work with tsx and some Node.js configurations due to CommonJS interop, it could fail in strict ES module environments or when bundled.
Consider using the ES module standard import.meta.resolve() (available in Node.js 20.6+) instead:
| ...(process.env.NODE_ENV === "development" && (() => { | |
| try { | |
| require.resolve("pino-pretty"); | |
| return { | |
| transport: { | |
| options: { | |
| colorize: true, | |
| ignore: "pid,hostname", | |
| }, | |
| target: "pino-pretty", | |
| }, | |
| }; | |
| } catch { | |
| // pino-pretty not available, use standard logging | |
| return {}; | |
| } | |
| })()), | |
| ...(process.env.NODE_ENV === "development" && (async () => { | |
| try { | |
| await import.meta.resolve("pino-pretty"); | |
| return { | |
| transport: { | |
| options: { | |
| colorize: true, | |
| ignore: "pid,hostname", | |
| }, | |
| target: "pino-pretty", | |
| }, | |
| }; | |
| } catch { | |
| // pino-pretty not available, use standard logging | |
| return {}; | |
| } | |
| })()), |
Note: This would require adjusting the Fastify configuration to handle the async resolution, or alternatively, use createRequire from node:module:
import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);
// Then use require.resolve() as beforePrompt To Fix With AI
This is a comment left during a code review.
Path: packages/server/src/server.ts
Line: 54:70
Comment:
Using `require.resolve()` in an ES module context (this package has `"type": "module"` in package.json) is non-standard. While it may work with `tsx` and some Node.js configurations due to CommonJS interop, it could fail in strict ES module environments or when bundled.
Consider using the ES module standard `import.meta.resolve()` (available in Node.js 20.6+) instead:
```suggestion
...(process.env.NODE_ENV === "development" && (async () => {
try {
await import.meta.resolve("pino-pretty");
return {
transport: {
options: {
colorize: true,
ignore: "pid,hostname",
},
target: "pino-pretty",
},
};
} catch {
// pino-pretty not available, use standard logging
return {};
}
})()),
```
Note: This would require adjusting the Fastify configuration to handle the async resolution, or alternatively, use `createRequire` from `node:module`:
```typescript
import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);
// Then use require.resolve() as before
```
How can I resolve this? If you propose a fix, please make it concise.1 participant
Why
Stagehand crashes in serverless environments (e.g. Vercel) when
pino-prettyis not available at runtime.pino-prettyis a dev-only formatter, but was treated as a required dependency, causing Pino to fail during initialization.What changed
pino-prettytooptionalDependenciespino-prettyusage withrequire.resolvechecksTest plan
This issue cannot be reliably reproduced locally due to Node.js module resolution and pnpm hoisting.
The fix targets serverless runtimes where optional dependencies are omitted from the runtime bundle.
Summary by cubic
Prevented serverless crashes by making pino-pretty optional and guarding its usage. Logging now falls back to standard Pino output when pino-pretty isn’t available.
Bug Fixes
Dependencies
Written for commit bcba3c5. Summary will update on new commits.