-
Type: Improvement
-
Resolution: Gone away
-
Priority: Major - P3
-
None
-
Affects Version/s: None
-
Component/s: Connection Layer
The MessageStream provides an abstraction that writes bytes to a socket, optionally compressing them, reads bytes from the socket, optionally decompresses them and batches them into "messages" - valid wire protocol responses.
Managing multiple streams is tricky and error prone (see the linked bug ticket). Additionally, our MessageStream abstraction blurs two different abstractions together - compression and response buffering.
There are alternate designs that could be simpler and less error prone (some rely on NodeJS 16+ APIs)
- Instead of piping the socket into the message stream and the message stream into the socket, wrap the socket inside the message stream. This reduces the number of streams the Connection manages and provides a better wire protocol message layer abstraction.
- Rely on NodeJS builtins to simplify managing multiple streams (such as "compose")
- Refactor away the message stream in favor of a compression stream and an async generator, which can be combined together using NodeJS's builtin "compose" method
We should consider alternative designs that are simpler to maintain and less prone to errors. Specifically, we should answer the following questions:
- How can we improve our MessageStream abstraction using modern javascript techniques and practices?
- How can we improve the reliability of the Connection class?
- How can we improve the unit-testability of our Connection layer and its components?
An example async iterator + compose implementation to achieve buffering without managing a custom stream:
async function* counter() { for (let i = 0; true; i++) { await setTimeout(500); yield i; } } async function* batch(source) { let batch: Array<number> = []; for await (const value of source) { batch.push(value); if (batch.length === 5) { yield batch; batch = []; } } } const input = Readable.from(counter()); for await (const value of compose(input, batch)) { console.log(value); }