typescript: support chunk indexes with empty message index #1304
Conversation
| @@ -38,10 +38,6 @@ export class ChunkCursor { | |||
| this.#startTime = params.startTime; | |||
| this.#endTime = params.endTime; | |||
| this.#reverse = params.reverse; | |||
|
|
|||
| if (this.chunkIndex.messageIndexLength === 0n) { | |||
There was a problem hiding this comment.
https://mcap.dev/spec#chunk-index-op0x08
I think it would be helpful/pedantic to confirm that the message start/end time are 0 and messageIndexOffsets is also empty? these should all be true if there are indeed no messages. Otherwise we should throw since that would mean there are messages but no message index (or an incorrect length for the index) which indicates malformed file from the POV of our app or spec.
There was a problem hiding this comment.
Added an extra check in loadMessageIndexes(). I think this is a better place to throw rather than in the constructur
There was a problem hiding this comment.
Why not throw as early as possible?
There was a problem hiding this comment.
What makes you say it is a better place to throw?
| @@ -38,10 +38,6 @@ export class ChunkCursor { | |||
| this.#startTime = params.startTime; | |||
| this.#endTime = params.endTime; | |||
| this.#reverse = params.reverse; | |||
|
|
|||
| if (this.chunkIndex.messageIndexLength === 0n) { | |||
There was a problem hiding this comment.
Why not throw as early as possible?
Changelog
typescript: support chunk indexes with empty message index
Docs
None
Description
When a chunk contains no messages, the corresponding ChunkIndex's message index is empty. Prior to this PR, the typescript lib threw an exception when a chunk index with an empty message index was encountered.
This PR changes this to no longer throw when such a chunk index is encountered. If the message index is empty, the internal method
#getSortTimewill fallback to the chunk index' start time which is0nand everything works as expected.mcap/typescript/core/src/ChunkCursor.ts
Lines 228 to 229 in e5c9da3
Note: We could change the
McapIndexedReaderto exclude such chunk indexes from its internal heap, but I believe that it's not worth adding additional code for improving performance of such a corner case