From 562538ed5d4587a52533beca2396a61efc6c98f3 Mon Sep 17 00:00:00 2001 From: Ari Perkkio Date: Sun, 7 Feb 2021 14:21:29 +0200 Subject: [PATCH] fix: handle results as streams - Large result sets crash JSON.parse and JSON.stringify calls - invalid string length - Use read & write streams to handle results - Use JSONStream to handle reading and writing JSON caches --- lib/JSONStream.d.ts | 5 ++ lib/file-client/result-comparator.ts | 98 +++++++++++++++++++--------- lib/file-client/results-writer.ts | 25 ++++--- lib/index.ts | 2 +- lib/progress-logger/exit-handler.ts | 4 +- package.json | 1 + test/unit/file-client.test.ts | 32 ++++----- yarn.lock | 2 +- 8 files changed, 110 insertions(+), 59 deletions(-) create mode 100644 lib/JSONStream.d.ts diff --git a/lib/JSONStream.d.ts b/lib/JSONStream.d.ts new file mode 100644 index 00000000..03eab188 --- /dev/null +++ b/lib/JSONStream.d.ts @@ -0,0 +1,5 @@ +// dominictarr/JSONStream#139 +declare module 'JSONStream' { + export function stringify(): NodeJS.ReadWriteStream; + export function parse(pattern: any): NodeJS.ReadWriteStream; +} diff --git a/lib/file-client/result-comparator.ts b/lib/file-client/result-comparator.ts index e5fe7331..9d0bd02b 100644 --- a/lib/file-client/result-comparator.ts +++ b/lib/file-client/result-comparator.ts @@ -1,4 +1,5 @@ import fs from 'fs'; +import * as JSONStream from 'JSONStream'; import { RESULTS_COMPARISON_CACHE_LOCATION, @@ -22,8 +23,10 @@ const EXTENSION = RESULT_PARSER_TO_EXTENSION[config.resultParser]; * - `added`: results which were present in previous scan but not in the current * - `removed`: results which are present in current scan but not in the previous */ -export function compareResults(current: Result[]): ComparisonResults { - const previous = readCache(); +export async function compareResults( + current: Result[] +): Promise { + const previous = await readCache(); if (previous.length === 0) { // All results are new @@ -49,39 +52,62 @@ export function compareResults(current: Result[]): ComparisonResults { /** * Write comparison results to file system and update cache with current results */ -export function writeComparisonResults( +export async function writeComparisonResults( comparisonResults: ComparisonResults, currentScanResults: Result[] -): void { - writeComparisons(comparisonResults); +): Promise { + await writeComparisons(comparisonResults); if (config.updateComparisonReference) { - updateCache(currentScanResults); + await updateCache(currentScanResults); } } -function readCache(): Result[] { +async function readCache(): Promise { if (!fs.existsSync(RESULTS_COMPARISON_CACHE_LOCATION)) { return []; } - const cache = fs.readFileSync(RESULTS_COMPARISON_CACHE_LOCATION, 'utf8'); - return JSON.parse(cache); + const results: Result[] = []; + + await new Promise((resolve, reject) => { + fs.createReadStream(RESULTS_COMPARISON_CACHE_LOCATION, { + encoding: 'utf8', + }) + .pipe(JSONStream.parse('*')) + .on('data', result => results.push(result)) + .on('end', resolve) + .on('error', reject); + }); + + return results; } -function updateCache(currentScanResults: Result[]): void { +async function updateCache(currentScanResults: Result[]): Promise { if (fs.existsSync(RESULTS_COMPARISON_CACHE_LOCATION)) { fs.unlinkSync(RESULTS_COMPARISON_CACHE_LOCATION); } - fs.writeFileSync( - RESULTS_COMPARISON_CACHE_LOCATION, - JSON.stringify(currentScanResults), - 'utf8' - ); + return new Promise((resolve, reject) => { + const stream = fs.createWriteStream(RESULTS_COMPARISON_CACHE_LOCATION); + const jstream = JSONStream.stringify() + .pipe(stream) + .on('finish', resolve) + .on('error', reject); + + const copy = [...currentScanResults]; + while (copy.length > 0) { + const chunk = copy.splice(0, 200); + jstream.write(JSON.stringify(chunk)); + } + jstream.end(); + stream.end(); + }); } -function writeComparisons(comparisonResults: ComparisonResults): void { +async function writeComparisons( + comparisonResults: ComparisonResults +): Promise { // Directory should always be available but let's handle condition where // user intentionally removes it during scan. if (!fs.existsSync(RESULTS_COMPARE_LOCATION)) { @@ -89,21 +115,31 @@ function writeComparisons(comparisonResults: ComparisonResults): void { } for (const type of ComparisonTypes) { - const results = comparisonResults[type]; - - if (results.length) { - const filename = `${type}${EXTENSION}`; - const content = [ - RESULT_COMPARISON_TEMPLATE.header(type), - ...results.map(RESULT_COMPARISON_TEMPLATE.results), - ].join('\n'); - - fs.writeFileSync( - `${RESULTS_COMPARE_LOCATION}/${filename}`, - content, - 'utf8' - ); - } + const results = [...comparisonResults[type]]; + if (!results.length) continue; + + await new Promise((resolve, reject) => { + const stream = fs + .createWriteStream( + `${RESULTS_COMPARE_LOCATION}/${type}${EXTENSION}`, + { encoding: 'utf8' } + ) + .on('finish', resolve) + .on('error', reject); + + stream.write(RESULT_COMPARISON_TEMPLATE.header(type)); + stream.write('\n'); + + while (results.length > 0) { + const chunk = results + .splice(0, 200) + .map(RESULT_COMPARISON_TEMPLATE.results) + .join('\n'); + stream.write(chunk); + } + + stream.end(); + }); } } diff --git a/lib/file-client/results-writer.ts b/lib/file-client/results-writer.ts index 8cc6aa26..0455e060 100644 --- a/lib/file-client/results-writer.ts +++ b/lib/file-client/results-writer.ts @@ -71,10 +71,10 @@ function parseMessages(messages: LintMessage[]): Result[] { /** * Write results to file at `./eslint-remote-tester-results` */ -export function writeResults( +export async function writeResults( messages: LintMessage[], repository: string -): void { +): Promise { // Don't write empty files for completely valid results if (!messages.length) { return; @@ -87,12 +87,21 @@ export function writeResults( // Construct result file name, e.g. mui-org_material-ui.md const repositoryOwnerAndName = repository.split('/').join('_'); const fileName = `${repositoryOwnerAndName}${RESULT_EXTENSION}`; - const formattedResults = results.map(RESULT_TEMPLATE).join('\n'); - fs.writeFileSync( - `${RESULTS_LOCATION}/${fileName}`, - formattedResults, - 'utf8' - ); + await new Promise((resolve, reject) => { + const stream = fs + .createWriteStream(`${RESULTS_LOCATION}/${fileName}`, { + encoding: 'utf8', + }) + .on('finish', resolve) + .on('error', reject); + + while (results.length > 0) { + const chunk = results.splice(0, 200); + const formattedResults = chunk.map(RESULT_TEMPLATE).join('\n'); + stream.write(formattedResults); + } + stream.end(); + }); } } diff --git a/lib/index.ts b/lib/index.ts index 368e9730..557533b3 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -118,7 +118,7 @@ async function scanRepo(repository: string) { ); try { - writeResults(results, repository); + await writeResults(results, repository); } catch (e) { logger.onWriteFailure(repository, e); } diff --git a/lib/progress-logger/exit-handler.ts b/lib/progress-logger/exit-handler.ts index 5409f1bb..8c74ff37 100644 --- a/lib/progress-logger/exit-handler.ts +++ b/lib/progress-logger/exit-handler.ts @@ -20,7 +20,7 @@ export default async function onExit(): Promise { if (config.compare) { try { - comparisonResults = compareResults(results); + comparisonResults = await compareResults(results); ResultsStore.setComparisonResults(comparisonResults); messages.push({ @@ -32,7 +32,7 @@ export default async function onExit(): Promise { level: 'verbose', }); - writeComparisonResults(comparisonResults, results); + await writeComparisonResults(comparisonResults, results); } catch (e) { errors.push('Error occured while generating comparison results'); errors.push(e.stack); diff --git a/package.json b/package.json index 7d0a7297..88e79ac6 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "bugs": "https://github.com/AriPerkkio/eslint-remote-tester", "dependencies": { "@babel/code-frame": "^7.12.13", + "JSONStream": "^1.3.5", "chalk": "^4.1.0", "ink": "^3.0.8", "object-hash": "^2.1.1", diff --git a/test/unit/file-client.test.ts b/test/unit/file-client.test.ts index d18be6f5..7adaa4a6 100644 --- a/test/unit/file-client.test.ts +++ b/test/unit/file-client.test.ts @@ -138,47 +138,47 @@ describe('file-client', () => { }); describe('compareResults', () => { - test('marks new results as "added"', () => { + test('marks new results as "added"', async () => { const one = generateResult('1'); const two = generateResult('2'); const three = generateResult('3'); createComparisonCache(three); - const results = compareResults([one, two, three]); + const results = await compareResults([one, two, three]); expect(results.added).toEqual([one, two]); expect(results.removed).toHaveLength(0); }); - test('marks disappeared results as "removed"', () => { + test('marks disappeared results as "removed"', async () => { const one = generateResult('1'); const two = generateResult('2'); const three = generateResult('3'); createComparisonCache(one, two, three); - const results = compareResults([three]); + const results = await compareResults([three]); expect(results.removed).toEqual([one, two]); expect(results.added).toHaveLength(0); }); - test('identifies changes in result.__internalHash', () => { + test('identifies changes in result.__internalHash', async () => { const result = generateResult(); createComparisonCache({ ...result, __internalHash: '1' as any }); - const results = compareResults([ + const results = await compareResults([ { ...result, __internalHash: '2' as any }, ]); expect(results.added).toEqual([{ ...result, __internalHash: '2' }]); }); - test('marks all results as "added" when previous results are not found', () => { + test('marks all results as "added" when previous results are not found', async () => { removeComparisonCache(); const one = generateResult('1'); const two = generateResult('2'); - const results = compareResults([one, two]); + const results = await compareResults([one, two]); expect(results.added).toEqual([one, two]); expect(results.removed).toHaveLength(0); @@ -186,7 +186,7 @@ describe('file-client', () => { }); describe('writeComparisonResults', () => { - test('writes comparison results to file system', () => { + test('writes comparison results to file system', async () => { const template = RESULT_PARSER_TO_COMPARE_TEMPLATE.markdown; const results = [generateResult('1'), generateResult('2')]; @@ -195,7 +195,7 @@ describe('file-client', () => { removed: [generateResult('4')], }; - writeComparisonResults(comparisonResults, results); + await writeComparisonResults(comparisonResults, results); const { added, removed } = getComparisonResults(); expect(added).toBe( @@ -210,31 +210,31 @@ describe('file-client', () => { ); }); - test('updates comparison cache on file system when updateComparisonReference is enabled', () => { + test('updates comparison cache on file system when updateComparisonReference is enabled', async () => { mockConfig.mockReturnValue({ updateComparisonReference: true }); removeComparisonCache(); const results = [generateResult('1'), generateResult('2')]; - writeComparisonResults({ added: [], removed: [] }, results); + await writeComparisonResults({ added: [], removed: [] }, results); expect(comparisonCacheExists()).toBe(true); expect(readComparisonCache()).toEqual(results); }); - test('does not update comparison cache when updateComparisonReference is disabled', () => { + test('does not update comparison cache when updateComparisonReference is disabled', async () => { mockConfig.mockReturnValue({ updateComparisonReference: false }); removeComparisonCache(); const results = [generateResult('1'), generateResult('2')]; - writeComparisonResults({ added: [], removed: [] }, results); + await writeComparisonResults({ added: [], removed: [] }, results); expect(comparisonCacheExists()).toBe(false); }); - test('does not crash if results compare location is not found', () => { + test('does not crash if results compare location is not found', async () => { removeResultsDirectory(); - writeComparisonResults({ added: [], removed: [] }, [ + await writeComparisonResults({ added: [], removed: [] }, [ generateResult(), ]); }); diff --git a/yarn.lock b/yarn.lock index 0888322a..855992d4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -949,7 +949,7 @@ "@typescript-eslint/types" "4.6.0" eslint-visitor-keys "^2.0.0" -JSONStream@^1.0.4: +JSONStream@^1.0.4, JSONStream@^1.3.5: version "1.3.5" resolved "https://registry.yarnpkg.com/JSONStream/-/JSONStream-1.3.5.tgz#3208c1f08d3a4d99261ab64f92302bc15e111ca0" integrity sha512-E+iruNOY8VV9s4JEbe1aNEm6MiszPRr/UfcHMz0TQh1BXSxHK+ASV1R6W4HpjBhSeS+54PIsAMCBmwD06LLsqQ==