fix(cli): fix bug where output gets truncated when too long#2862
Conversation
|
@MikeRalphson @P0lip could someone please review this PR? |
|
@barryadk could you please take a look at this PR? |
Co-authored-by: knieczyszczak <46347253+knieczyszczak@users.noreply.github.com>
…railing whitespace on lines 62 and 65.
There was a problem hiding this comment.
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
## [6.15.1](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-cli-6.15.0...@stoplight/spectral-cli-6.15.1) (2026-04-13) ### Bug Fixes * **cli:** fix bug where output gets truncated when too long ([#2862](#2862)) ([0e6fd33](0e6fd33)) * **cli:** throw error if no file found to lint ([#2778](#2778)) ([3e20072](3e20072)) * **core:** fix security issue of simple-eval package ([#2886](#2886)) ([8120a76](8120a76)) * **core:** respect off severity in intermediate rulesets ([#2890](#2890)) ([5b99b99](5b99b99)) * **formatters:** Fix rendering of github actions documentationUrl ([#2895](#2895)) ([df27b06](df27b06)) * **formatters:** markdown formatter with invalid-ref crashes spectral ([#2905](#2905)) ([59728e4](59728e4)) * **functions:** export or function ([#2812](#2812)) ([03532a5](03532a5)) * **repo:** release step marking repo as save for git ([#2884](#2884)) ([87147a6](87147a6)) * **repo:** remove acceptance step on release ([#2882](#2882)) ([73496c6](73496c6)) * **ruleset-migrator:** fix ruleset migrator output when a rule name contains '/' ([#2859](#2859)) ([115d1d0](115d1d0)) * **rulesets:** use uri-reference for openIdConnectUrl ([#2796](#2796)) ([c57eb59](c57eb59)) ### Features * **core:** allow extending rulesets with aliases ([#2870](#2870)) ([8db9718](8db9718)) * **core:** further adjustments for extending rulesets with aliases ([#2939](#2939)) ([26144bc](26144bc)) * **repo:** circleci migration to GHA (OP-35885) ([#2867](#2867)) ([884f079](884f079)) * **repo:** npm release workflow as gha ([#2880](#2880)) ([0147d6e](0147d6e)) * **repo:** post develop merge workflow ([#2877](#2877)) ([9420713](9420713)) * **repo:** replace skypack usage with esm cdn ([#2940](#2940)) ([0d6a910](0d6a910))
# [1.22.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-core-1.21.0...@stoplight/spectral-core-1.22.0) (2026-04-13) ### Bug Fixes * **cli:** fix bug where output gets truncated when too long ([#2862](#2862)) ([0e6fd33](0e6fd33)) * **cli:** throw error if no file found to lint ([#2778](#2778)) ([3e20072](3e20072)) * **core:** fix security issue of simple-eval package ([#2886](#2886)) ([8120a76](8120a76)) * **core:** respect off severity in intermediate rulesets ([#2890](#2890)) ([5b99b99](5b99b99)) * **formatters:** Fix rendering of github actions documentationUrl ([#2895](#2895)) ([df27b06](df27b06)) * **formatters:** markdown formatter with invalid-ref crashes spectral ([#2905](#2905)) ([59728e4](59728e4)) * **rulesets:** use uri-reference for openIdConnectUrl ([#2796](#2796)) ([c57eb59](c57eb59)) ### Features * **core:** further adjustments for extending rulesets with aliases ([#2939](#2939)) ([26144bc](26144bc)) * **repo:** replace skypack usage with esm cdn ([#2940](#2940)) ([0d6a910](0d6a910))
## [1.5.1](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-formatters-1.5.0...@stoplight/spectral-formatters-1.5.1) (2026-04-13) ### Bug Fixes * **cli:** fix bug where output gets truncated when too long ([#2862](#2862)) ([0e6fd33](0e6fd33)) * **cli:** throw error if no file found to lint ([#2778](#2778)) ([3e20072](3e20072)) * **core:** fix security issue of simple-eval package ([#2886](#2886)) ([8120a76](8120a76)) * **core:** respect off severity in intermediate rulesets ([#2890](#2890)) ([5b99b99](5b99b99)) * **formatters:** Fix rendering of github actions documentationUrl ([#2895](#2895)) ([df27b06](df27b06)) * **formatters:** markdown formatter with invalid-ref crashes spectral ([#2905](#2905)) ([59728e4](59728e4)) * **functions:** export or function ([#2812](#2812)) ([03532a5](03532a5)) * **repo:** release step marking repo as save for git ([#2884](#2884)) ([87147a6](87147a6)) * **repo:** remove acceptance step on release ([#2882](#2882)) ([73496c6](73496c6)) * **ruleset-migrator:** fix ruleset migrator output when a rule name contains '/' ([#2859](#2859)) ([115d1d0](115d1d0)) * **rulesets:** use uri-reference for openIdConnectUrl ([#2796](#2796)) ([c57eb59](c57eb59)) ### Features * **core:** allow extending rulesets with aliases ([#2870](#2870)) ([8db9718](8db9718)) * **core:** further adjustments for extending rulesets with aliases ([#2939](#2939)) ([26144bc](26144bc)) * **repo:** circleci migration to GHA (OP-35885) ([#2867](#2867)) ([884f079](884f079)) * **repo:** npm release workflow as gha ([#2880](#2880)) ([0147d6e](0147d6e)) * **repo:** post develop merge workflow ([#2877](#2877)) ([9420713](9420713)) * **repo:** replace skypack usage with esm cdn ([#2940](#2940)) ([0d6a910](0d6a910))
|
🎉 This PR is included in version 6.15.1 🎉 The release is available on Your semantic-release bot 📦🚀 |
# [1.12.0](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-ruleset-migrator-1.11.3...@stoplight/spectral-ruleset-migrator-1.12.0) (2026-04-13) ### Bug Fixes * **cli:** fix bug where output gets truncated when too long ([#2862](#2862)) ([0e6fd33](0e6fd33)) * **cli:** throw error if no file found to lint ([#2778](#2778)) ([3e20072](3e20072)) * **core:** fix security issue of simple-eval package ([#2886](#2886)) ([8120a76](8120a76)) * **core:** respect off severity in intermediate rulesets ([#2890](#2890)) ([5b99b99](5b99b99)) * **formatters:** Fix rendering of github actions documentationUrl ([#2895](#2895)) ([df27b06](df27b06)) * **formatters:** markdown formatter with invalid-ref crashes spectral ([#2905](#2905)) ([59728e4](59728e4)) * **repo:** release step marking repo as save for git ([#2884](#2884)) ([87147a6](87147a6)) * **repo:** remove acceptance step on release ([#2882](#2882)) ([73496c6](73496c6)) * **rulesets:** use uri-reference for openIdConnectUrl ([#2796](#2796)) ([c57eb59](c57eb59)) ### Features * **core:** allow extending rulesets with aliases ([#2870](#2870)) ([8db9718](8db9718)) * **core:** further adjustments for extending rulesets with aliases ([#2939](#2939)) ([26144bc](26144bc)) * **repo:** circleci migration to GHA (OP-35885) ([#2867](#2867)) ([884f079](884f079)) * **repo:** npm release workflow as gha ([#2880](#2880)) ([0147d6e](0147d6e)) * **repo:** post develop merge workflow ([#2877](#2877)) ([9420713](9420713)) * **repo:** replace skypack usage with esm cdn ([#2940](#2940)) ([0d6a910](0d6a910))
## [1.22.1](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-rulesets-1.22.0...@stoplight/spectral-rulesets-1.22.1) (2026-04-13) ### Bug Fixes * **cli:** fix bug where output gets truncated when too long ([#2862](#2862)) ([0e6fd33](0e6fd33)) * **cli:** throw error if no file found to lint ([#2778](#2778)) ([3e20072](3e20072)) * **core:** fix security issue of simple-eval package ([#2886](#2886)) ([8120a76](8120a76)) * **core:** respect off severity in intermediate rulesets ([#2890](#2890)) ([5b99b99](5b99b99)) * **formatters:** Fix rendering of github actions documentationUrl ([#2895](#2895)) ([df27b06](df27b06)) * **formatters:** markdown formatter with invalid-ref crashes spectral ([#2905](#2905)) ([59728e4](59728e4)) * **functions:** export or function ([#2812](#2812)) ([03532a5](03532a5)) * **repo:** release step marking repo as save for git ([#2884](#2884)) ([87147a6](87147a6)) * **repo:** remove acceptance step on release ([#2882](#2882)) ([73496c6](73496c6)) * **ruleset-migrator:** fix ruleset migrator output when a rule name contains '/' ([#2859](#2859)) ([115d1d0](115d1d0)) * **rulesets:** use uri-reference for openIdConnectUrl ([#2796](#2796)) ([c57eb59](c57eb59)) ### Features * **core:** allow extending rulesets with aliases ([#2870](#2870)) ([8db9718](8db9718)) * **core:** further adjustments for extending rulesets with aliases ([#2939](#2939)) ([26144bc](26144bc)) * **repo:** circleci migration to GHA (OP-35885) ([#2867](#2867)) ([884f079](884f079)) * **repo:** npm release workflow as gha ([#2880](#2880)) ([0147d6e](0147d6e)) * **repo:** post develop merge workflow ([#2877](#2877)) ([9420713](9420713)) * **repo:** replace skypack usage with esm cdn ([#2940](#2940)) ([0d6a910](0d6a910))
## [1.1.5](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-runtime-1.1.4...@stoplight/spectral-runtime-1.1.5) (2026-04-14) ### Bug Fixes * **cli:** fix bug where output gets truncated when too long ([#2862](#2862)) ([0e6fd33](0e6fd33)) * **cli:** throw error if no file found to lint ([#2778](#2778)) ([3e20072](3e20072)) * **core:** fix security issue of simple-eval package ([#2886](#2886)) ([8120a76](8120a76)) * **core:** respect off severity in intermediate rulesets ([#2890](#2890)) ([5b99b99](5b99b99)) * **deps:** bump elliptic from 6.6.0 to 6.6.1 ([#2782](#2782)) ([5ff9602](5ff9602)) * **deps:** fix yarn lockfile ([e6c3b9d](e6c3b9d)) * **deps:** post lodash update changes for semantic release ([#2942](#2942)) ([bf530dd](bf530dd)) * **deps:** update spectral-core in cli ([35687cd](35687cd)) * **formatters:** Fix rendering of github actions documentationUrl ([#2895](#2895)) ([df27b06](df27b06)) * **formatters:** markdown formatter with invalid-ref crashes spectral ([#2905](#2905)) ([59728e4](59728e4)) * **functions:** export or function ([#2812](#2812)) ([03532a5](03532a5)) * **repo:** release step marking repo as save for git ([#2884](#2884)) ([87147a6](87147a6)) * **repo:** remove acceptance step on release ([#2882](#2882)) ([73496c6](73496c6)) * **repo:** replace discord link with forum link ([#2793](#2793)) ([6229442](6229442)) * **ruleset-migrator:** fix ruleset migrator output when a rule name contains '/' ([#2859](#2859)) ([115d1d0](115d1d0)) * **rulesets:** use uri-reference for openIdConnectUrl ([#2796](#2796)) ([c57eb59](c57eb59)) ### Features * **core:** add documentUrl to JS api and cli formatters ([#2443](#2443)) ([e787728](e787728)) * **core:** allow extending rulesets with aliases ([#2870](#2870)) ([8db9718](8db9718)) * **core:** further adjustments for extending rulesets with aliases ([#2939](#2939)) ([26144bc](26144bc)) * **functions:** add or function ([#2798](#2798)) ([d9ef27f](d9ef27f)) * **repo:** circleci migration to GHA (OP-35885) ([#2867](#2867)) ([884f079](884f079)) * **repo:** npm release workflow as gha ([#2880](#2880)) ([0147d6e](0147d6e)) * **repo:** post develop merge workflow ([#2877](#2877)) ([9420713](9420713)) * **repo:** replace skypack usage with esm cdn ([#2940](#2940)) ([0d6a910](0d6a910))
Fixes #2861.
Checklist
Note
Does this PR introduce a breaking change?
Additional context
See this issue. #2861