Skip to content

fix(cli): fix bug where output gets truncated when too long#2862

Merged
knieczyszczak merged 5 commits intostoplightio:developfrom
sidneyamani:fix-trucated-output
Jan 30, 2026
Merged

fix(cli): fix bug where output gets truncated when too long#2862
knieczyszczak merged 5 commits intostoplightio:developfrom
sidneyamani:fix-trucated-output

Conversation

@sidneyamani
Copy link
Contributor

@sidneyamani sidneyamani commented Oct 29, 2025

Fixes #2861.

Checklist

  • Tests added / updated -> Test fixed. Adding a test would require generating a 2MB input file. Let me know if you'd like me to do that.
  • Docs added / updated -> In-code comment explain why.

Note
Does this PR introduce a breaking change?

  • Yes
  • No

Additional context

See this issue. #2861

@sidneyamani sidneyamani requested a review from a team as a code owner October 29, 2025 04:27
@sidneyamani
Copy link
Contributor Author

@MikeRalphson @P0lip could someone please review this PR?

@sidneyamani
Copy link
Contributor Author

@barryadk could you please take a look at this PR?

@tomek-tursa-sb tomek-tursa-sb changed the title Fix bug where output gets truncated when too long fix(cli): Fix bug where output gets truncated when too long Nov 18, 2025
@tomek-tursa-sb tomek-tursa-sb changed the title fix(cli): Fix bug where output gets truncated when too long fix(cli): fix bug where output gets truncated when too long Nov 18, 2025
Copilot AI added a commit that referenced this pull request Jan 29, 2026
Co-authored-by: knieczyszczak <46347253+knieczyszczak@users.noreply.github.com>
Copy link
Collaborator

@knieczyszczak knieczyszczak left a comment

Choose a reason for hiding this comment

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

PR #2862 Review: Fix bug where output gets truncated when too long
Summary
✅ RECOMMENDATION: APPROVE AND MERGE

This PR fixes a legitimate bug where large SARIF output (~1MB+) gets truncated when piped to stdout. The fix is minimal, correct, and well-tested.

Problem Analysis
The Bug
When Spectral CLI outputs large results (e.g., SARIF format with ~200 paths = ~1MB output), the output gets truncated because:

process.stdout.write() is a non-blocking operation
When writing large amounts of data, Node.js may apply backpressure
The original code didn't wait for the write to complete or handle backpressure
The function returns immediately while data is still being written
When the process exits, any unflushed data in the buffer is lost
Root Cause
// Original code - PROBLEMATIC
export async function writeOutput(outputStr: string, outputFile: string): Promise {
if (outputFile !== '') {
await fs.writeFile(outputFile, outputStr);
} else {
process.stdout.write(outputStr); // Returns immediately, doesn't wait for flush
}
}
The issue occurs when:

Large output is written to stdout
The write operation returns false (indicating backpressure)
The function returns immediately without waiting
Process exits before all data is flushed
Solution Analysis
The Fix
// Fixed code - CORRECT
export async function writeOutput(outputStr: string, outputFile: string): Promise {
if (outputFile !== '') {
await fs.writeFile(outputFile, outputStr);
} else {
// Handle backpressure by using the callback parameter
// The callback is invoked when the data is flushed (or an error occurs)
return new Promise((resolve, reject) => {
process.stdout.write(outputStr, err => {
if (err) reject(err);
else resolve();
});
});
}
}
Why This Works
Callback-based approach: Uses process.stdout.write()'s callback parameter
Promisification: Wraps the callback in a Promise to maintain async/await compatibility
Proper error handling: Rejects on errors, resolves on success
Backpressure handling: The callback is only invoked after data is flushed
Consistent with fs.writeFile: Both branches now properly await completion
Code Quality Assessment
✅ Strengths
Minimal change: Only changes what's necessary
Well-commented: Explains why the callback is needed
Proper error handling: Catches and propagates errors
Test coverage: Updated test to mock the callback properly
Consistent API: Maintains async/await pattern throughout
No breaking changes: Function signature and behavior remain the same for callers
Test Coverage
The PR updates the test to properly mock process.stdout.write with callback support:

const writeMock = jest.spyOn(process.stdout, 'write').mockImplementation((chunk: any, callback?: any) => {
if (typeof callback === 'function') {
callback();
}
return true;
});

expect(await writeOutput(output, '')).toBeUndefined();
expect(writeMock).toHaveBeenCalledWith(output, expect.any(Function));

writeMock.mockRestore();
This test:

✅ Properly mocks the callback parameter
✅ Verifies the callback is passed
✅ Cleans up the mock after the test
✅ Tests the async behavior correctly
Verification
Manual Testing Results
I tested both the broken and fixed versions:

Without fix (develop branch):

❌ FAIL: SARIF output truncated
With fix (PR branch):

✅ PASS: SARIF output pipes correctly (979508 bytes)
The fix successfully handles ~1MB of output without truncation.

Automated Testing
All existing tests pass:

PASS @stoplight/spectral-cli packages/cli/src/services/tests/output.test.ts
Test Suites: 1 passed, 1 total
Tests: 5 passed, 5 total
Performance Considerations
Impact: Minimal to None
Small outputs: No measurable difference (callback overhead is negligible)
Large outputs: Slightly slower but correct (must wait for flush)
Files: No change (already used await with fs.writeFile)
The performance impact is acceptable because:

Correctness > Speed (truncated output is useless)
The overhead is minimal for typical use cases
Large outputs already had performance constraints
Security Considerations
No Security Issues
✅ No new dependencies
✅ No external inputs
✅ Proper error handling
✅ No code execution paths
✅ No data leakage
Compatibility
Backward Compatible
✅ No breaking changes
✅ Function signature unchanged
✅ Behavior unchanged for small outputs
✅ Fixes behavior for large outputs (previously broken)
Documentation
Adequate Documentation
✅ In-code comments explain the fix
✅ PR description references the issue
✅ No user-facing docs needed (internal fix)
Recommendations
Primary Recommendation: ✅ MERGE
This PR should be merged because:

Fixes a real bug: Demonstrated with reproduction case
Minimal risk: Small, focused change
Well-tested: Updated tests, all passing
No breaking changes: Fully backward compatible
Good code quality: Clear, commented, follows best practices
Optional: Future Enhancements
While not necessary for this PR, future improvements could include:

Chunked writing: For extremely large outputs (>10MB), could write in chunks
Progress indication: For very large files, show progress
Stream-based API: Replace string concatenation with streaming for memory efficiency
However, these are NOT blockers for this PR and would be separate enhancements.

Conclusion
APPROVE AND MERGE ✅

This PR successfully fixes a legitimate bug where large Spectral output gets truncated. The fix is:

Minimal and focused
Technically correct
Well-tested
Backward compatible
Properly documented
There are no blocking issues or concerns. The PR is ready to merge.

Testing Evidence
Test Run: develop (without fix)
❌ FAIL: SARIF output truncated
Test Run: pr-2862 (with fix)
✅ PASS: SARIF output pipes correctly (979508 bytes)
Unit Tests
PASS @stoplight/spectral-cli packages/cli/src/services/tests/output.test.ts
Output service
formatOutput
✓ calls stylish formatter with given result
✓ calls json formatter with given result
✓ calls junit formatter with given result
writeOutput
✓ given outputFile, writes output to a specified path
✓ given , print output to console

Test Suites: 1 passed, 1 total
Tests: 5 passed, 5 total

https://github.com/stoplightio/spectral/tasks/97458b77-85c2-4d0c-8368-5f41365eb015

@knieczyszczak knieczyszczak merged commit 0e6fd33 into stoplightio:develop Jan 30, 2026
8 checks passed
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.

SARIF outputs gets truncated when input files are too long

3 participants