Skip to content

fix: include stdout in commit() error when stderr is empty - #19

Merged
404-Page-Found merged 1 commit into
404-PF:mainfrom
MohammadYusif:fix/issue-17
May 27, 2026
Merged

fix: include stdout in commit() error when stderr is empty#19
404-Page-Found merged 1 commit into
404-PF:mainfrom
MohammadYusif:fix/issue-17

Conversation

@MohammadYusif

Copy link
Copy Markdown
Contributor

Summary

When git commit fails with no stderr output, the thrown error message was just "git commit exited with code 1" with no useful detail. This fix captures both stderr and stdout so the full git output is always included in the error.

Why this change

spawnSync returns both result.stderr and result.stdout, but the original code only fell back to result.stderr. Git sometimes writes failure output to stdout instead of stderr (e.g. pre-commit hook failures, nothing to commit messages), leaving the error completely blank for the user.

What changed

  • src/git/diff.ts: join result.stderr and result.stdout, filter empty strings, trim — use the combined output as the error detail
  • Falls back to the generic git commit exited with code N message only if both streams are empty

Validation

  • npm run build — clean, no TypeScript errors
  • Manually verified the change reads both output streams before constructing the error

User-facing impact

  • affect scanning behavior
  • affect output formatting
  • affect JSON output
  • affect docs only

Error message shown to the user on a failed commit is now more informative. No behavior change on the happy path.

Notes

No tests exist in this repo yet. The fix is a one-line logic change in a small, focused function.

When git commit fails with no stderr output, the error message was
just 'git commit exited with code 1' with no useful detail. git
sometimes writes failure output to stdout instead of stderr.

Join both stderr and stdout (filtering empty strings) so the full
git output is always surfaced in the thrown error.

Closes 404-PF#17

@404-Page-Found 404-Page-Found left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR Review: fix: include stdout in commit() error when stderr is empty

Overview

Fixes a bug where failed git commit calls could produce unhelpful error messages when stderr is empty but stdout contains failure details (e.g., pre-commit hook output). Combines both streams before constructing the error, falling back to the generic message only if both are empty.

Code Quality

  • Clean, idiomatic pattern (filter(Boolean).join('\n').trim()) consistent with the existing getCombinedDiff() function on line 44.
  • Minimal, well-scoped change — +2/−1 lines, no new dependencies.
  • Readable and maintainable.

Potential Issues

  • Whitespace-only streams: Handled correctly — .trim() on the joined result means the detail || ... fallback catches effectively empty strings.
  • No tests: Acknowledged by the author. Acceptable for this scope.

Suggestions

  • No blocking suggestions. The code is fine as-is.

Verdict

✅ Approve — Correct, well-documented, follows project conventions.

@404-Page-Found
404-Page-Found merged commit 92f0625 into 404-PF:main May 27, 2026
MohammadYusif added a commit to MohammadYusif/commit-echo that referenced this pull request May 28, 2026
fix: include stdout in commit() error when stderr is empty
@MohammadYusif
MohammadYusif deleted the fix/issue-17 branch May 28, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants