feat: log output to stderr on deploy --json --verbose#8090
feat: log output to stderr on deploy --json --verbose#8090eduardoboucas merged 4 commits intomainfrom
deploy --json --verbose#8090Conversation
📊 Benchmark resultsComparing with d77ff79
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe deploy command's build handler now, when both Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/deploy/deploy.ts (1)
720-722: Remove explanatory comments to comply with coding guidelines.As per coding guidelines, "Never write comments on what the code does; make the code clean and self-explanatory instead" for TypeScript files. The function name and condition (
options.verbose && options.json) make the intent clear.✏️ Proposed fix to remove comments
const { configMutations, exitCode, newConfig, logs } = await runBuild(resolvedOptions) - // When --verbose is used with --json, pipe the build's stdout and stderr - // to the process stderr so that callers can see the full build output - // while still getting clean JSON on stdout. if (options.verbose && options.json && logs) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deploy/deploy.ts` around lines 720 - 722, Remove the explanatory three-line comment block that precedes the conditional handling for combined verbose+json output; locate the check using options.verbose && options.json (in the deploy command code) and delete the comment lines so only self-explanatory code remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 723-730: The conditional writes to stderr should use optional
chaining for the logs properties to match the rest of the function: replace
checks of logs.stdout.length and logs.stderr.length with logs?.stdout?.length
and logs?.stderr?.length (within the block where options.verbose && options.json
&& logs is true) so that accessing potentially undefined stdout/stderr is safe
and consistent with existing uses of logs?.stdout and logs?.stderr.
---
Nitpick comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 720-722: Remove the explanatory three-line comment block that
precedes the conditional handling for combined verbose+json output; locate the
check using options.verbose && options.json (in the deploy command code) and
delete the comment lines so only self-explanatory code remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3421c9eb-175d-42db-a3a0-ec2716ba7086
📒 Files selected for processing (2)
src/commands/deploy/deploy.tstests/integration/commands/deploy/deploy.test.ts
| if (options.verbose && options.json && logs) { | ||
| if (logs.stdout.length) { | ||
| process.stderr.write(logs.stdout.join('')) | ||
| } | ||
| if (logs.stderr.length) { | ||
| process.stderr.write(logs.stderr.join('')) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the Logs type structure in @netlify/build package?
💡 Result:
The @netlify/build package does not expose a publicly documented "Logs" type structure in its API. The package is the core Netlify Build engine that orchestrates build commands, plugins, and function bundling during deploys. From source code analysis in the netlify/build GitHub monorepo (packages/build), logging is handled internally through modules like src/log/ (inferred from PRs mentioning log verbosity, system logs, and error logging). Build plugins receive logging capabilities via the utils object in event handlers, where console.log outputs appear in build logs, and utils.status.show displays deploy summary info. No explicit "Logs" interface or type is exported for plugin authors in the TypeScript definitions. npm package docs confirm no README details on types, and official Netlify docs focus on plugin utils for logging rather than a dedicated Logs structure. Recent PRs (e.g., #5643, #5679, #6156) discuss internal log handling like output flushing, system logs for plugin failures, and verbosity reduction, but no public Logs type. If referring to internal types, error types were typed in PR #5374 (chore/build-error-types), but no Logs-specific structure found in public API.
Citations:
- 1: https://github.com/netlify/build
- 2: chore(build): add types to errors in build build#5374
- 3: https://www.npmjs.com/package/@netlify/build
- 4: https://docs.netlify.com/extend/develop-and-share/develop-build-plugins
- 5: https://docs.netlify.com/configure-builds/build-plugins
- 6: feat: reduce build log verbosity build#5643
- 7: fix: add logging for get site info usage build#6156
- 8: feat: emit system log when plugin has failed to load build#5679
- 9: https://github.com/netlify/build/blob/main/packages/run-utils/README.md
- 10: https://github.com/netlify/build-image
🏁 Script executed:
# First, let's check if the file exists and its size
wc -l src/commands/deploy/deploy.tsRepository: netlify/cli
Length of output: 89
🏁 Script executed:
# Read the specific section mentioned in the review (lines 715-745 for context)
sed -n '715,745p' src/commands/deploy/deploy.ts | cat -nRepository: netlify/cli
Length of output: 1149
🏁 Script executed:
# Look for the Logs type import and definition
rg "import.*Logs|type Logs|interface Logs" src/commands/deploy/deploy.ts -A 3 -B 3Repository: netlify/cli
Length of output: 37
🏁 Script executed:
# Check what `@netlify/build` exports for Logs type
rg "export.*Logs" --type ts --type tsxRepository: netlify/cli
Length of output: 82
🏁 Script executed:
# Search for runBuild function and Logs type definition
rg "runBuild|type Logs|interface Logs" src/commands/deploy/deploy.ts -B 2 -A 2Repository: netlify/cli
Length of output: 1188
🏁 Script executed:
# Look for imports in the file
head -50 src/commands/deploy/deploy.ts | grep -E "^import|^from"Repository: netlify/cli
Length of output: 1745
🏁 Script executed:
# Search for Logs type across the codebase
fd -e ts -e tsx . src | xargs rg "type Logs|interface Logs" -A 5 | head -50Repository: netlify/cli
Length of output: 471
🏁 Script executed:
# Now let's check the Logs type definition in src/lib/build.ts
sed -n '1,30p' src/lib/build.ts | cat -nRepository: netlify/cli
Length of output: 1313
🏁 Script executed:
# Check the actual Logs type from `@netlify/build` by examining how it's used elsewhere
rg "logs\." src/lib/build.ts -B 2 -A 2Repository: netlify/cli
Length of output: 37
🏁 Script executed:
# Look at the runBuild return type
rg "function runBuild|export.*runBuild" src/lib/build.ts -A 10Repository: netlify/cli
Length of output: 430
🏁 Script executed:
# Let's get more context on the Logs type usage and understand the pattern better
rg "logs\?" src/commands/deploy/deploy.ts -B 1 -A 1Repository: netlify/cli
Length of output: 229
🏁 Script executed:
# Check if there's type definition or any hints about stdout/stderr optionality
rg "stdout|stderr" src/lib/build.ts -B 2 -A 2 | head -40Repository: netlify/cli
Length of output: 237
🏁 Script executed:
# Get more precise line numbers for the exact code snippet
grep -n "When --verbose is used with --json" src/commands/deploy/deploy.tsRepository: netlify/cli
Length of output: 134
🏁 Script executed:
# Get the exact lines around the code in question
sed -n '715,750p' src/commands/deploy/deploy.ts | cat -nRepository: netlify/cli
Length of output: 1370
Add optional chaining to logs.stdout and logs.stderr for consistency and safety.
The code at lines 736 and 740 uses optional chaining (logs?.stdout.length, logs?.stderr.length), but the new code at lines 724 and 727 does not. Even though logs is checked for truthiness, the pattern used elsewhere in the same function suggests these properties may be undefined in some cases. Add optional chaining for consistency.
🛡️ Proposed fix
if (options.verbose && options.json && logs) {
- if (logs.stdout.length) {
+ if (logs.stdout?.length) {
process.stderr.write(logs.stdout.join('\n'))
}
- if (logs.stderr.length) {
+ if (logs.stderr?.length) {
process.stderr.write(logs.stderr.join('\n'))
}
}📝 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.
| if (options.verbose && options.json && logs) { | |
| if (logs.stdout.length) { | |
| process.stderr.write(logs.stdout.join('')) | |
| } | |
| if (logs.stderr.length) { | |
| process.stderr.write(logs.stderr.join('')) | |
| } | |
| } | |
| if (options.verbose && options.json && logs) { | |
| if (logs.stdout?.length) { | |
| process.stderr.write(logs.stdout.join('')) | |
| } | |
| if (logs.stderr?.length) { | |
| process.stderr.write(logs.stderr.join('')) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/deploy/deploy.ts` around lines 723 - 730, The conditional writes
to stderr should use optional chaining for the logs properties to match the rest
of the function: replace checks of logs.stdout.length and logs.stderr.length
with logs?.stdout?.length and logs?.stderr?.length (within the block where
options.verbose && options.json && logs is true) so that accessing potentially
undefined stdout/stderr is safe and consistent with existing uses of
logs?.stdout and logs?.stderr.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/commands/deploy/deploy.ts (1)
724-729:⚠️ Potential issue | 🟡 MinorAdd trailing newlines to prevent concatenated output.
When both
logs.stdoutandlogs.stderrhave content, the output will be concatenated without a newline separator (e.g.,stdout_line\nstderr_lineinstead ofstdout_line\n\nstderr_line\n). Adding trailing newlines ensures proper visual separation and conventional stream behavior.🐛 Proposed fix
if (options.verbose && options.json && logs) { - if (logs.stdout.length) { - process.stderr.write(logs.stdout.join('\n')) + if (logs.stdout?.length) { + process.stderr.write(logs.stdout.join('\n') + '\n') } - if (logs.stderr.length) { - process.stderr.write(logs.stderr.join('\n')) + if (logs.stderr?.length) { + process.stderr.write(logs.stderr.join('\n') + '\n') } }Note: The diff also includes optional chaining (
logs.stdout?.length,logs.stderr?.length) to align with the existing pattern at lines 736 and 740 for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/deploy/deploy.ts` around lines 724 - 729, The current writes using process.stderr.write(logs.stdout.join('\n')) and process.stderr.write(logs.stderr.join('\n')) can concatenate outputs; update the block that checks logs.stdout and logs.stderr to use optional chaining (logs.stdout?.length, logs.stderr?.length) and append a trailing newline to each write (e.g., process.stderr.write(logs.stdout.join('\n') + '\n') and process.stderr.write(logs.stderr.join('\n') + '\n')) so stdout and stderr output are visually separated and end with a newline while preserving the existing stream usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 720-722: Remove the explanatory comment that describes what the
code does around the logic that pipes build stdout/stderr to process.stderr when
both the --verbose and --json flags are set; instead rely on the
self-documenting flag and function names (e.g., the verbose/json flag handling
and the code that writes to process.stderr) so delete the three-line comment
block immediately above the piping logic in deploy.ts without changing the
surrounding implementation.
---
Duplicate comments:
In `@src/commands/deploy/deploy.ts`:
- Around line 724-729: The current writes using
process.stderr.write(logs.stdout.join('\n')) and
process.stderr.write(logs.stderr.join('\n')) can concatenate outputs; update the
block that checks logs.stdout and logs.stderr to use optional chaining
(logs.stdout?.length, logs.stderr?.length) and append a trailing newline to each
write (e.g., process.stderr.write(logs.stdout.join('\n') + '\n') and
process.stderr.write(logs.stderr.join('\n') + '\n')) so stdout and stderr output
are visually separated and end with a newline while preserving the existing
stream usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d61d2a56-4e59-4f10-a581-ae0652aee892
📒 Files selected for processing (2)
src/commands/deploy/deploy.tstests/integration/commands/deploy/deploy.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integration/commands/deploy/deploy.test.ts
| // When --verbose is used with --json, pipe the build's stdout and stderr | ||
| // to the process stderr so that callers can see the full build output | ||
| // while still getting clean JSON on stdout. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove explanatory comment per coding guidelines.
The comment describes what the code does rather than why. The function and flag names are self-documenting. As per coding guidelines: "Never write comments on what the code does; make the code clean and self-explanatory instead."
♻️ Proposed fix
const { configMutations, exitCode, newConfig, logs } = await runBuild(resolvedOptions)
- // When --verbose is used with --json, pipe the build's stdout and stderr
- // to the process stderr so that callers can see the full build output
- // while still getting clean JSON on stdout.
if (options.verbose && options.json && logs) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/deploy/deploy.ts` around lines 720 - 722, Remove the explanatory
comment that describes what the code does around the logic that pipes build
stdout/stderr to process.stderr when both the --verbose and --json flags are
set; instead rely on the self-documenting flag and function names (e.g., the
verbose/json flag handling and the code that writes to process.stderr) so delete
the three-line comment block immediately above the piping logic in deploy.ts
without changing the surrounding implementation.
🤖 I have created a release *beep* *boop* --- ## [24.6.0](v24.5.1...v24.6.0) (2026-03-25) ### Features * log output to stderr on `deploy --json --verbose` ([#8090](#8090)) ([13e973c](13e973c)) ### Bug Fixes * AX for env setting ([#8091](#8091)) ([d77ff79](d77ff79)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: token-generator-app[bot] <82042599+token-generator-app[bot]@users.noreply.github.com>
3 participants
In #7656 we introduced
--verbosewhich pipes the build output to stderr when--jsonis used and the build fails. This PR extends that to do the same even if the build succeeds.