Skip to content

Commit 3657add

Browse files
TheLarkInnCopilot
andcommitted
Preserve NDJSON records across decode errors
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ea5daa0b-7839-42f0-9d63-4ca84c7d2b5c
1 parent 719230e commit 3657add

4 files changed

Lines changed: 98 additions & 8 deletions

File tree

common/reviews/api/rush-reporter.api.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,17 @@ export class NdjsonDecoder {
261261
flush(): unknown[];
262262
}
263263

264+
// @beta
265+
export class NdjsonInvalidRecordError extends Error {
266+
constructor(decodedRecords: readonly unknown[], cause: Error);
267+
readonly cause: Error;
268+
readonly decodedRecords: readonly unknown[];
269+
}
270+
264271
// @beta
265272
export class NdjsonRecordTooLargeError extends Error {
266-
constructor(maxRecordBytes: number);
273+
constructor(maxRecordBytes: number, decodedRecords?: readonly unknown[]);
274+
readonly decodedRecords: readonly unknown[];
267275
readonly maxRecordBytes: number;
268276
}
269277

libraries/reporter/src/index.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,12 @@ export {
7777
isReporterProtocolCompatible
7878
} from './protocol/ReporterProtocol';
7979
export type { INdjsonOptions } from './protocol/Ndjson';
80-
export { NdjsonRecordTooLargeError, encodeNdjsonRecord, NdjsonDecoder } from './protocol/Ndjson';
80+
export {
81+
NdjsonInvalidRecordError,
82+
NdjsonRecordTooLargeError,
83+
encodeNdjsonRecord,
84+
NdjsonDecoder
85+
} from './protocol/Ndjson';
8186
export type {
8287
IReporterHello,
8388
IReporterHelloAck,

libraries/reporter/src/protocol/Ndjson.ts

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,49 @@ export class NdjsonRecordTooLargeError extends Error {
1414
*/
1515
public readonly maxRecordBytes: number;
1616

17-
public constructor(maxRecordBytes: number) {
17+
/**
18+
* Valid records completed by the decoder before it rejected the oversized record.
19+
*/
20+
public readonly decodedRecords: readonly unknown[];
21+
22+
public constructor(maxRecordBytes: number, decodedRecords: readonly unknown[] = []) {
1823
super(`The NDJSON record exceeds the maximum size of ${maxRecordBytes} bytes.`);
1924
this.name = 'NdjsonRecordTooLargeError';
2025
this.maxRecordBytes = maxRecordBytes;
26+
this.decodedRecords = [...decodedRecords];
2127

2228
// Restore the prototype chain, which is broken when subclassing a built-in
2329
// and compiling to CommonJS.
2430
Object.setPrototypeOf(this, NdjsonRecordTooLargeError.prototype);
2531
}
2632
}
2733

34+
/**
35+
* Thrown when a completed NDJSON record is not valid JSON.
36+
*
37+
* @beta
38+
*/
39+
export class NdjsonInvalidRecordError extends Error {
40+
/**
41+
* Valid records completed before the malformed record.
42+
*/
43+
public readonly decodedRecords: readonly unknown[];
44+
45+
/**
46+
* The JSON parser error that caused this failure.
47+
*/
48+
public readonly cause: Error;
49+
50+
public constructor(decodedRecords: readonly unknown[], cause: Error) {
51+
super('The NDJSON record is not valid JSON.');
52+
this.name = 'NdjsonInvalidRecordError';
53+
this.decodedRecords = [...decodedRecords];
54+
this.cause = cause;
55+
56+
Object.setPrototypeOf(this, NdjsonInvalidRecordError.prototype);
57+
}
58+
}
59+
2860
/**
2961
* Options controlling NDJSON record size enforcement.
3062
*
@@ -67,6 +99,9 @@ export function encodeNdjsonRecord(value: unknown, options?: INdjsonOptions): st
6799
* Call {@link NdjsonDecoder.decode} for each received chunk to obtain the
68100
* records completed by that chunk, then call {@link NdjsonDecoder.flush} once
69101
* the stream ends to obtain any trailing record that was not newline-terminated.
102+
* If a later record fails, its error exposes earlier valid records through
103+
* `decodedRecords`; the rejected completed line is consumed and any subsequent
104+
* buffered lines remain available to a later call.
70105
*
71106
* @beta
72107
*/
@@ -83,7 +118,8 @@ export class NdjsonDecoder {
83118
* Appends a chunk and returns any records it completed.
84119
*
85120
* @param chunk - a fragment of the NDJSON stream
86-
* @throws NdjsonRecordTooLargeError if a record exceeds the limit
121+
* @throws {@link NdjsonRecordTooLargeError} if a record exceeds the limit
122+
* @throws {@link NdjsonInvalidRecordError} if a completed record is malformed
87123
*/
88124
public decode(chunk: string): unknown[] {
89125
this._buffer += chunk;
@@ -99,7 +135,7 @@ export class NdjsonDecoder {
99135

100136
// A partial line that already exceeds the limit can never become a valid record.
101137
if (Buffer.byteLength(this._buffer, 'utf8') > this._maxRecordBytes) {
102-
throw new NdjsonRecordTooLargeError(this._maxRecordBytes);
138+
throw new NdjsonRecordTooLargeError(this._maxRecordBytes, records);
103139
}
104140

105141
return records;
@@ -108,7 +144,8 @@ export class NdjsonDecoder {
108144
/**
109145
* Returns any trailing record that was not newline-terminated and resets the buffer.
110146
*
111-
* @throws NdjsonRecordTooLargeError if the trailing record exceeds the limit
147+
* @throws {@link NdjsonRecordTooLargeError} if the trailing record exceeds the limit
148+
* @throws {@link NdjsonInvalidRecordError} if the trailing record is malformed
112149
*/
113150
public flush(): unknown[] {
114151
const records: unknown[] = [];
@@ -122,12 +159,18 @@ export class NdjsonDecoder {
122159

123160
private _processLine(line: string, records: unknown[]): void {
124161
if (Buffer.byteLength(line, 'utf8') > this._maxRecordBytes) {
125-
throw new NdjsonRecordTooLargeError(this._maxRecordBytes);
162+
throw new NdjsonRecordTooLargeError(this._maxRecordBytes, records);
126163
}
127164
const trimmed: string = line.trim();
128165
if (trimmed.length === 0) {
129166
return;
130167
}
131-
records.push(JSON.parse(trimmed));
168+
try {
169+
records.push(JSON.parse(trimmed));
170+
} catch (error) {
171+
const cause: Error =
172+
error instanceof Error ? error : new Error('An unknown JSON parsing failure occurred.');
173+
throw new NdjsonInvalidRecordError(records, cause);
174+
}
132175
}
133176
}

libraries/reporter/src/test/Protocol.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
isReporterProtocolCompatible,
88
encodeNdjsonRecord,
99
NdjsonDecoder,
10+
NdjsonInvalidRecordError,
1011
NdjsonRecordTooLargeError,
1112
InvalidReporterHelloError,
1213
negotiateReporterHello,
@@ -71,6 +72,39 @@ describe('NDJSON encode/decode', () => {
7172
const decoder: NdjsonDecoder = new NdjsonDecoder({ maxRecordBytes: 10 });
7273
expect(() => decoder.decode('x'.repeat(50))).toThrow(NdjsonRecordTooLargeError);
7374
});
75+
76+
it('exposes valid records completed before a malformed later record', () => {
77+
const decoder: NdjsonDecoder = new NdjsonDecoder();
78+
let caught: unknown;
79+
try {
80+
decoder.decode('{"id":1}\nnot-json\n{"id":2}\n');
81+
} catch (error) {
82+
caught = error;
83+
}
84+
85+
expect(caught).toBeInstanceOf(NdjsonInvalidRecordError);
86+
if (!(caught instanceof NdjsonInvalidRecordError)) {
87+
throw new Error('Expected an NdjsonInvalidRecordError.');
88+
}
89+
expect(caught.decodedRecords).toEqual([{ id: 1 }]);
90+
expect(decoder.decode('')).toEqual([{ id: 2 }]);
91+
});
92+
93+
it('exposes valid records completed before an oversized later record', () => {
94+
const decoder: NdjsonDecoder = new NdjsonDecoder({ maxRecordBytes: 10 });
95+
let caught: unknown;
96+
try {
97+
decoder.decode(`1\n${'"'}${'x'.repeat(20)}${'"'}\n`);
98+
} catch (error) {
99+
caught = error;
100+
}
101+
102+
expect(caught).toBeInstanceOf(NdjsonRecordTooLargeError);
103+
if (!(caught instanceof NdjsonRecordTooLargeError)) {
104+
throw new Error('Expected an NdjsonRecordTooLargeError.');
105+
}
106+
expect(caught.decodedRecords).toEqual([1]);
107+
});
74108
});
75109

76110
describe('negotiateReporterHello', () => {

0 commit comments

Comments
 (0)