fix: Avoid out of bounds error. #125
Conversation
Signed-off-by: Paolo Insogna <paolo@cowtech.it>
| partition.records.push(readRecordsBatch(recordsBatchesReader)) | ||
| try { | ||
| partition.records.push(readRecordsBatch(recordsBatchesReader)) | ||
| } catch (err) { |
There was a problem hiding this comment.
I'm not sure it works. When the error happens, if the reader position is not moved backwards, the already read records will be lost
There was a problem hiding this comment.
Nope, that's not how it works.
The already read records have been added. Only the truncated part is discarded.
There was a problem hiding this comment.
So I think the record in between of the truncation will be lost
There was a problem hiding this comment.
Which is fine because on the next iteration Kafka will not send the rest of the message but rather start from the beginning.
| ok(Buffer.isBuffer(record.value)) | ||
| }) | ||
|
|
||
| test('parseResponse handles truncated records', () => { |
There was a problem hiding this comment.
We should test when the truncation happens in the nth recordsBatch
There was a problem hiding this comment.
It already does. It fails in the 2nd batch.
There was a problem hiding this comment.
I don't think this test the error. When the truncation happens in partition.recordsBatch we should properly read all the records, so for example:
- record 1, truncation on record 2, record 3
we should get all the 3 records
There was a problem hiding this comment.
See above, if the record is truncated, we discard and in the next run we get it from the beginning, including the already transmitted truncated part.
Based of the work of @simone-sanfratello in #124.
Fixes #120.