Write [Shakapacker] log messages to stderr instead of stdout #869

justin808 · 2026-01-21T04:57:00Z

Summary

Fixes #868. This PR changes all informational [Shakapacker] log messages to be written to stderr instead of stdout.

When running bin/shakapacker --profile --json, the log messages were being written to stdout, which broke JSON parsing:

[Shakapacker] Looking for Webpack config in: /app/config/webpack/webpack.config.ts, /app/config/webpack/webpack.config.js
[Shakapacker] Found Webpack config: /app/config/webpack/webpack.config.js
[Shakapacker] Preparing environment for assets bundler execution...
{"hash":"b52b191c34c1f1b3e6a9","version":"5.99.9",...}

This caused SyntaxError: Unexpected token 'S', "[Shakapacke"... is not valid JSON.

Changes

Changed all puts "[Shakapacker]..." statements to $stderr.puts "[Shakapacker]..." in:
- lib/shakapacker/runner.rb
- lib/shakapacker/dev_server_runner.rb
Updated corresponding test files to capture stderr instead of stdout for log message assertions

Behavior After This PR

Log messages are written to stderr and remain visible to users
stdout only contains the actual webpack/rspack output (including JSON when using --json)
Tools parsing stdout (like webpack-bundle-analyzer) receive valid JSON

Test plan

All runner-related tests pass (126 examples, 0 failures)
RuboCop passes
Manual testing: bin/shakapacker --profile --json > stats.json should produce valid JSON

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

Refactor
- Logging and informational messages from the build and dev-server processes are now directed to standard error instead of standard output. This improves output stream organization for better compatibility with standard Unix tooling and may affect custom scripts or workflows that capture or redirect build output.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-21T04:57:23Z

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 2 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 8ff75d5 and 265dfe1.

📒 Files selected for processing (5)

lib/shakapacker/dev_server_runner.rb
lib/shakapacker/runner.rb
spec/shakapacker/rspack_runner_spec.rb
spec/shakapacker/runner_build_config_spec.rb
spec/shakapacker/webpack_runner_spec.rb

Walkthrough

This PR redirects logging and progress messages from stdout to stderr across the runner and dev server runner to prevent informational output from polluting JSON output when using the --json flag. Production code and corresponding tests are updated to capture from stderr instead.

Changes

Cohort / File(s)	Summary
Runner Output Redirection `lib/shakapacker/runner.rb`, `lib/shakapacker/dev_server_runner.rb`	Redirects all logging, progress, and informational messages from `$stdout`/`puts` to `$stderr.puts`, including config discovery, build details, environment setup notices, and completion messages. Control flow unchanged.
Test Output Capturing `spec/shakapacker/runner_build_config_spec.rb`, `spec/shakapacker/rspack_runner_spec.rb`, `spec/shakapacker/webpack_runner_spec.rb`	Updates test helpers and assertions to capture from `$stderr` instead of `$stdout`. Removes `capture_stdout` helper method and replaces all occurrences with `capture_stderr` to match production code behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Improve error messages to suggest assets_bundler_config_path #712: Refactors config-not-found error printing and formatting in runner.rb—related as both PRs modify error/informational output handling in the same file.
Add timing logs for webpack and rspack builds #706: Modifies logging behavior in runner.rb and dev_server_runner.rb with timing logs and execution changes—related due to overlapping production files and output stream modifications.

Poem

🐰 A stream of messages, once so bright,
Now flows to stderr, where it's right!
Stdout stays pure, for JSON's sake,
One less broken parse for you to make! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: redirecting log messages from stdout to stderr.
Linked Issues check	✅ Passed	The PR fully implements the preferred solution from issue `#868` by writing all log messages to stderr instead of stdout, ensuring clean JSON output.
Out of Scope Changes check	✅ Passed	All changes are scoped to redirecting log output streams in runner files and updating corresponding test assertions; no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch jg/fix-issue-868

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

greptile-apps · 2026-01-21T04:58:46Z

Greptile Summary

This PR fixes a critical bug where [Shakapacker] informational log messages were being written to stdout instead of stderr, breaking JSON output parsing when using --json flag.

Key Changes:

Changed ~24 puts statements to $stderr.puts in lib/shakapacker/runner.rb and lib/shakapacker/dev_server_runner.rb
Updated all corresponding tests to capture stderr instead of stdout
All log messages with [Shakapacker] prefix now correctly go to stderr

Impact:

Fixes issue bin/shakapacker --profile --json outputs log messages to stdout, breaking JSON parsing #868 where bin/shakapacker --profile --json output was unparsable
stdout now contains only webpack/rspack output (including valid JSON when --json is used)
Log messages remain visible to users on stderr
Tools like webpack-bundle-analyzer can now successfully parse the JSON output

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The changes are straightforward and surgical - only changing output stream from stdout to stderr for log messages. All tests have been updated accordingly and the fix directly addresses the reported issue without introducing new logic or complexity.
No files require special attention

Important Files Changed

Filename	Overview
lib/shakapacker/runner.rb	Changed 20 informational log messages from `puts` to `$stderr.puts` to prevent JSON output contamination. All logging statements correctly updated.
lib/shakapacker/dev_server_runner.rb	Updated 4 informational log messages to use `$stderr.puts` for consistency with runner.rb changes. Aligns dev server logging with main runner.
spec/shakapacker/webpack_runner_spec.rb	Updated test helper from `capture_stdout` to `capture_stderr` to match the logging changes. Tests now correctly assert stderr output.
spec/shakapacker/rspack_runner_spec.rb	Updated test helper from `capture_stdout` to `capture_stderr` to match logging changes. Maintains test coverage for rspack runner.
spec/shakapacker/runner_build_config_spec.rb	Updated all test assertions to use `capture_stderr` and removed duplicate `capture_stdout` helper. Tests properly validate stderr logging behavior.

Sequence Diagram

sequenceDiagram
    participant User
    participant Runner
    participant Webpack/Rspack
    participant stdout
    participant stderr

    User->>Runner: bin/shakapacker --profile --json
    Runner->>stderr: [Shakapacker] Looking for config...
    Runner->>stderr: [Shakapacker] Found config
    Runner->>stderr: [Shakapacker] Preparing environment...
    Runner->>stderr: [Shakapacker] Base command: ...
    Runner->>stderr: [Shakapacker] Final command: ...
    Runner->>Webpack/Rspack: Execute bundler command
    Webpack/Rspack->>stdout: {"hash":"...","version":"...",...}
    Runner->>stderr: [Shakapacker] Completed build in X.XXs
    stdout->>User: Valid JSON (no log messages)
    stderr->>User: Log messages visible in terminal

claude · 2026-01-21T04:58:53Z

Code Review for PR #869

Overall Assessment

This PR successfully addresses issue #868 by fixing the JSON parsing problem when using bin/shakapacker --profile --json. The approach is sound and the implementation is clean. ✅

Positive Findings

1. Code Quality

✅ Consistent changes across all affected files
✅ The fix correctly separates informational logs (stderr) from actual output (stdout)
✅ Follows Ruby best practices for using $stderr vs $stdout
✅ No breaking changes to functionality - logs remain visible to users

2. Test Coverage

✅ All test files properly updated to capture stderr instead of stdout
✅ Tests verify the timing output format in both regular and watch modes
✅ Good test coverage for edge cases (watch mode, elapsed time formatting)
✅ Helper method capture_stderr correctly implemented in all spec files

3. Standards Compliance

✅ Follows project conventions from CLAUDE.md
✅ Changes are focused and minimal
✅ No extraneous modifications

4. Performance & Security

✅ No performance impact - just redirecting output stream
✅ No security concerns introduced

Issues Found

1. Incomplete Migration ⚠️

Found 4 additional puts statements with [Shakapacker] prefix that should be migrated to $stderr.puts for consistency in lib/shakapacker/runner.rb at lines 483-484 and 511-512 in the init_config_file and list_builds methods.

2. Other stdout Usage ℹ️

For your awareness, there are other uses of $stdout.puts in lib/shakapacker/dev_server_runner.rb and lib/tasks/shakapacker/*.rake files that appear to be error messages. These should likely also use $stderr.puts but are outside the scope of this PR.

Recommendations

Required for this PR:

Migrate the 4 remaining puts statements in runner.rb to use $stderr.puts
Run bundle exec rubocop to ensure linting passes
Run the full test suite with bundle exec rspec

Optional follow-up:

Consider a separate PR to migrate all error/warning messages to stderr
Add an integration test that verifies bin/shakapacker --json produces valid JSON

Summary

This is a well-executed fix for a real issue. The only blocker is completing the migration for the remaining 4 puts statements to ensure consistency. Once that's addressed, this PR will be ready to merge.

Great work on identifying and fixing this issue! 🎉

claude · 2026-01-21T05:00:04Z

Code Review

This PR successfully addresses issue #868 by redirecting all [Shakapacker] log messages to stderr, ensuring stdout contains only the actual webpack/rspack output. This is a well-executed fix with proper test coverage.

✅ Strengths

Code Quality:

Comprehensive coverage: All 27 instances of [Shakapacker] log messages were correctly updated across both runner.rb and dev_server_runner.rb
Consistent implementation: The change from puts to $stderr.puts is applied uniformly
Proper separation of concerns: Deprecation warnings already used $stderr.puts, and now all informational logs follow the same pattern

Best Practices:

Unix philosophy compliance: Separating diagnostic output (stderr) from program output (stdout) is correct Unix behavior
Tooling compatibility: Fixes JSON parsing issues for tools like webpack-bundle-analyzer that consume stdout
User experience preserved: Log messages remain visible to users; only the stream changed

Test Coverage:

Thorough test updates: All affected test files properly updated to capture stderr instead of stdout
Test helper cleanup: Removed duplicate capture_stdout helper in runner_build_config_spec.rb:280-289 (was redundant since capture_stderr already existed)
Correct test assertions: Tests now verify the right stream, ensuring logs go where expected

🔍 Potential Improvements

Minor consideration (not blocking):

Line 29 in runner.rb: $stdout.sync = true is still present. While this doesn't cause issues, it's now less relevant since most output goes to stderr. Consider whether $stderr.sync = true should also be added for consistency in real-time log output (not required for this PR).
Manual testing checkbox: The PR description shows manual testing is unchecked. Before merging, consider manually verifying bin/shakapacker --profile --json > stats.json produces valid JSON.

📊 Impact Assessment

Performance: ✅ No impact
Security: ✅ No concerns
Breaking changes: ✅ None (output destination change is transparent to users viewing terminal)
Backward compatibility: ✅ Excellent (users see the same messages, scripts parsing stdout get cleaner output)

🎯 Recommendation

Approve and merge after manual testing verification. This is a high-quality fix that:

Solves a real bug (bin/shakapacker --profile --json outputs log messages to stdout, breaking JSON parsing #868)
Follows Unix conventions correctly
Has comprehensive test coverage
Makes no breaking changes to user-facing behavior

Great work on this fix! 🚀

Reviewed with attention to: CLAUDE.md guidelines, RSpec testing patterns, Ruby style consistency

Fixes #868. When using `bin/shakapacker --profile --json`, the [Shakapacker] log messages were being written to stdout, which broke JSON parsing tools like webpack-bundle-analyzer. Changed all informational log messages (prefixed with [Shakapacker]) to use $stderr.puts instead of puts, so that: - stdout remains clean for JSON output from webpack/rspack - log messages are still visible to users on stderr - tools that parse stdout get valid JSON Updated test files to capture stderr instead of stdout for tests that verify log output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

claude · 2026-01-21T05:03:38Z

Code Review

✅ Overall Assessment

This is a well-executed fix that correctly addresses issue #868. The changes are focused, consistent, and properly tested.

🎯 Strengths

Correct Solution: Writing informational log messages to stderr is the right approach. This follows Unix conventions where stdout is for primary output (JSON in this case) and stderr is for logging/diagnostics.
Comprehensive Coverage: All [Shakapacker] informational messages have been consistently changed from puts to $stderr.puts across both production files.
Test Updates: All corresponding test files were properly updated to capture stderr instead of stdout, maintaining test coverage.
Code Consistency: The change aligns with existing codebase patterns - I verified that errors and deprecation warnings already use $stderr.puts throughout the codebase.
No Breaking Changes: The log messages remain visible to users (stderr is displayed in terminals by default), so this won't break existing workflows.

📋 Detailed Review

Production Code Changes ✅

lib/shakapacker/runner.rb: 23 lines updated correctly
lib/shakapacker/dev_server_runner.rb: 4 lines updated correctly

All informational messages now use $stderr.puts, including:

Build configuration details
Command preparation steps
Config file discovery messages
Build completion timings

Test Changes ✅

spec/shakapacker/rspack_runner_spec.rb: capture_stdout → capture_stderr (3 occurrences)
spec/shakapacker/webpack_runner_spec.rb: capture_stdout → capture_stderr (3 occurrences)
spec/shakapacker/runner_build_config_spec.rb: capture_stdout → capture_stderr (6 occurrences), removed duplicate helper

The test helper consolidation in runner_build_config_spec.rb is a nice cleanup - removing the now-unused capture_stdout helper while keeping capture_stderr.

🔍 Code Quality

No Issues Found

✅ No security concerns
✅ No performance implications
✅ No bugs introduced
✅ Test coverage maintained (all log message assertions updated)
✅ Follows project conventions (per CLAUDE.md)
✅ Consistent with existing error/warning patterns in the codebase

🧪 Testing Recommendations

The PR mentions manual testing as incomplete. I recommend:

# Verify JSON output is clean
bin/shakapacker --profile --json > stats.json
cat stats.json | jq .  # Should parse without errors

# Verify logs still appear on console
bin/shakapacker 2>&1 | grep "\[Shakapacker\]"

This will confirm that:

JSON output to stdout is clean and parseable
Log messages to stderr are still visible when needed

📝 Minor Suggestion (Optional)

Consider adding a brief note in the commit message or code comment explaining why stderr is used, for future maintainers:

# Log to stderr to keep stdout clean for machine-readable output (e.g., --json flag)
$stderr.puts "[Shakapacker] Running build: #{build_config[:name]}"

This is optional but could prevent future confusion.

✅ Recommendation

LGTM - Approve and Merge

This PR successfully fixes the reported issue without introducing any regressions. The implementation is clean, well-tested, and follows best practices.

claude · 2026-01-21T05:04:13Z

Code Review

Summary

This PR successfully addresses issue #868 by redirecting Shakapacker log messages from stdout to stderr, allowing tools like webpack-bundle-analyzer to properly parse JSON output from --json flag. The changes are well-focused and consistent.

Strengths ✅

Correct Problem Identification: The fix directly addresses the root cause - informational log messages were polluting stdout and breaking JSON parsing.
Consistent Implementation: All [Shakapacker] log messages in the build execution path are consistently changed from puts to $stderr.puts.
Proper Test Updates: Test files correctly updated to capture stderr instead of stdout using capture_stderr helper, which aligns with the new behavior.
Good Test Coverage: Tests verify that build completion messages appear in stderr and that watch mode doesn't show completion messages.
Clean Refactoring: Removed duplicate capture_stdout helper in runner_build_config_spec.rb (lines 280-289 in old version) since only capture_stderr is now needed.

Issues & Recommendations 🔍

1. Inconsistent stderr Usage for Version/Help Output

Issue: The --version and --help outputs still use plain puts instead of $stderr.puts:

lib/shakapacker/runner.rb:467-475 - print_version uses puts
lib/shakapacker/dev_server_runner.rb:217-225 - print_version uses puts

Question: Should --version output go to stdout or stderr?

Standard Unix convention: --version typically outputs to stdout (e.g., node --version, ruby --version), allowing shakapacker --version | grep -oE '[0-9]+' to work.
Current PR approach: Leaves version on stdout, which seems correct.

Recommendation: The current approach is fine. Version and help information should remain on stdout per Unix conventions. However, consider documenting this decision.

2. Other stdout Messages Not in Scope

Several other puts statements remain that write to stdout:

lib/shakapacker/dev_server_runner.rb:

Line 255-256: Error messages ("webpack 'dev_server' configuration not found")
Line 265: Unsupported CLI switches error
Line 270: HTTPS configuration error
Line 280: Port conflict error

lib/shakapacker/runner.rb:

Line 483-484: Config file exists message
Line 511: No config file found message

lib/shakapacker/doctor.rb:

Multiple puts statements for diagnostic output (lines 848, 901-902, 1022, 1044)

Assessment: These are error messages or user-facing command outputs (like doctor), not build log messages. They should arguably stay on stdout or be moved to stderr as errors. This is outside the scope of the current PR but worth noting.

Recommendation: Consider a follow-up issue to audit all output streams for consistency:

Errors → stderr
Warnings → stderr
Informational build logs → stderr
Command output (version, help, doctor diagnostics) → stdout
Build artifacts (JSON, stats) → stdout

3. Missing Changelog Entry

The project guidelines indicate:

Update CHANGELOG.md for user-visible changes only (features, bug fixes, breaking changes, deprecations, performance improvements)

This is a bug fix (JSON output was broken) and changes observable behavior (log output stream), so it should have a changelog entry.

Recommendation: Add a changelog entry:

### Fixed
- [PR #869](https://github.com/shakacode/shakapacker/pull/869): Write [Shakapacker] log messages to stderr instead of stdout by [justin808](https://github.com/justin808) - Fixes JSON output parsing when using `--json` flag

Or use the /update-changelog command per CLAUDE.md guidelines.

4. Test Coverage Gap

The tests verify that log messages appear in stderr, but they don't verify that stdout is clean when using --json flag.

Recommendation: Consider adding a test that:

Runs with --json flag
Captures stdout separately
Verifies stdout contains valid JSON (parseable)
Verifies [Shakapacker] messages are NOT in stdout

Example:

it "produces valid JSON in stdout without log messages when using --json" do
  # Mock system call and verify JSON output is clean
end

This would prevent regression of the exact bug being fixed.

Code Quality ✅

Style: Changes follow existing code conventions
Consistency: $stderr.puts used consistently throughout modified code
Test Quality: Proper use of RSpec mocking and helpers
No Over-Engineering: Minimal, focused changes

Security & Performance ✅

No security concerns
No performance impact (stderr vs stdout writing is equivalent)

Conclusion

This is a solid bug fix that solves the immediate problem. The implementation is clean and well-tested.

Recommended Actions Before Merge:

✅ RuboCop passes (per checklist)
✅ Tests pass (per checklist)
⚠️ Add CHANGELOG.md entry (missing)
⚠️ Consider adding JSON validation test to prevent regression
💡 Optional: Document the stdout/stderr convention in code comments or docs

Overall: Approve with minor suggestions - The core fix is correct, just needs changelog entry.

Review generated by Claude Code

coderabbitai bot approved these changes Jan 21, 2026

View reviewed changes

justin808 force-pushed the jg/fix-issue-868 branch from 8ff75d5 to 265dfe1 Compare January 21, 2026 05:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Write [Shakapacker] log messages to stderr instead of stdout #869

Write [Shakapacker] log messages to stderr instead of stdout #869

justin808 commented Jan 21, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 21, 2026 •

edited

Loading

Rate limit exceeded

Uh oh!

greptile-apps bot commented Jan 21, 2026

Uh oh!

claude bot commented Jan 21, 2026

Uh oh!

claude bot commented Jan 21, 2026

Uh oh!

claude bot commented Jan 21, 2026

Uh oh!

claude bot commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Write [Shakapacker] log messages to stderr instead of stdout #869

Are you sure you want to change the base?

Write [Shakapacker] log messages to stderr instead of stdout #869

Conversation

justin808 commented Jan 21, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Behavior After This PR

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

greptile-apps bot commented Jan 21, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

claude bot commented Jan 21, 2026

Code Review for PR #869

Overall Assessment

Positive Findings

Issues Found

Recommendations

Summary

Uh oh!

claude bot commented Jan 21, 2026

Code Review

✅ Strengths

🔍 Potential Improvements

📊 Impact Assessment

🎯 Recommendation

Uh oh!

claude bot commented Jan 21, 2026

Code Review

✅ Overall Assessment

🎯 Strengths

📋 Detailed Review

Production Code Changes ✅

Test Changes ✅

🔍 Code Quality

🧪 Testing Recommendations

📝 Minor Suggestion (Optional)

✅ Recommendation

Uh oh!

claude bot commented Jan 21, 2026

Code Review

Summary

Strengths ✅

Issues & Recommendations 🔍

1. Inconsistent stderr Usage for Version/Help Output

2. Other stdout Messages Not in Scope

3. Missing Changelog Entry

4. Test Coverage Gap

Code Quality ✅

Security & Performance ✅

Conclusion

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

justin808 commented Jan 21, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 21, 2026 •

edited

Loading