JS: new Quality query - Unhandled errors in .pipe() chain
#19544
Conversation
…error handlers in `Node.js` streams
Add tests for chained stream methods and non-stream pipe objects
…ld be flagged by the query
26f0aeb to
21aa9d1
Compare
…fter pipe operations
…om `pipe()` calls based on accessed properties
…pipe method detection
…ied as `pipe` calls on streams
…and `javascript-code-quality.qls`
21aa9d1 to
b104871
Compare
…t flag such instances.
…n and some popular library ones
…n one passes function as second arg to `pipe`
1fba9e0 to
b10a948
Compare
… from being flagged by unhandled pipe error query
|
QHelp previews: javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.qhelpUnhandled error in stream pipelineIn Node.js, calling the RecommendationInstead of using If you must use ExampleThe following code snippet demonstrates a problematic usage of the const fs = require('fs');
const source = fs.createReadStream('source.txt');
const A better approach is to use the const { pipeline } = require('stream');
const fs = require('fs');
const source = fs.createReadStream('source.txt');
const destination = fs.createWriteStream('destination.txt');
// Good: Using pipeline for automatic error handling
pipeline(
source,
destination,
(err) => {
if (err) {
console.error('Pipeline failed:', err);
} else {
console.log('Pipeline succeeded');
}
}
);Alternatively, if you need to use const fs = require('fs');
const source = fs.createReadStream('source.txt');
const destination = fs.createWriteStream('destination.txt');
// Alternative Good: Manual error handling with pipe()
source.on('error', (err) => {
console.error('Source stream error:', err);
destination.destroy(err);
});
destination.on('error', (err) => {
console.error('Destination stream error:', err);
source.destroy(err);
});
source.pipe(destination);References
|
Co-authored-by: Asger F <asgerf@github.com>
Co-authored-by: Asger F <asgerf@github.com>
Co-authored-by: Asger F <asgerf@github.com>
Co-authored-by: Asger F <asgerf@github.com>
Co-authored-by: Asger F <asgerf@github.com>
8e95d06 to
d43695c
Compare
Co-authored-by: Asger F <asgerf@github.com>
Address comments Co-Authored-By: Asger F <316427+asgerf@users.noreply.github.com>
| * @id js/nodejs-stream-pipe-without-error-handling | ||
| * @name Node.js stream pipe without error handling | ||
| * @description Calling `pipe()` on a stream without error handling will drop errors coming from the input stream | ||
| * @kind problem |
There was a problem hiding this comment.
| * @id js/nodejs-stream-pipe-without-error-handling | |
| * @name Node.js stream pipe without error handling | |
| * @description Calling `pipe()` on a stream without error handling will drop errors coming from the input stream | |
| * @kind problem | |
| * @id js/unhandled-error-in-stream-pipeline | |
| * @name Unhandled error in stream pipeline | |
| * @description Calling `pipe()` on a stream without error handling will drop errors coming from the input stream | |
| * @kind problem |
- I think we should leave out
nodejsfrom the query name and ID. It might grow in scope to cover other kinds of streams. - "Without error handling" sounds a bit off, since there is some error handling, it's just that some of the errors slip through.
Also, if you agree with the query name and ID change, could you rename the .ql files to match afterwards?
There was a problem hiding this comment.
That makes a lot of sense to me! Changed it here 8521c53, there was a small side effect in expected files due to the merge, but it was expected.
Co-authored-by: Asger F <asgerf@github.com>
As a side effect of merge `security-and-quality` does not contain anymore related new query. Co-Authored-By: Asger F <316427 EE99 +asgerf@users.noreply.github.com>
| @@ -1,2 +1,2 @@ | |||
| query: Quality/UnhandledStreamPipe.ql | |||
There was a problem hiding this comment.
Let's also rename the test folder.
And rename the .qlref file to UnhandledErrorInStreamPipeline.qlref as per convention.
There was a problem hiding this comment.
Renamed here 8521c53
Co-Authored-By: Asger F <316427+asgerf@users.noreply.github.com>
This pull request adds a new quality query for
JavaScript. This query checks for unhandled errors in the.pipe()chain of Node.js streams, helping to ensure that potential stream errors are not missed and applications remain robust.