-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Make WebSocket Reader/Writer (#5001) #5002
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - nice tests!
async read(p: Uint8Array): Promise<number | null> { | ||
for await (const ev of this.receive()) { | ||
if (ev instanceof Uint8Array) { | ||
return copyBytes(p, ev); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copyBytes
ignores overflowed data of src. It seems to discard data in frames, doesn't it?.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are right. Would something like this make sense instead?
#readBacklog = new Uint8Array();
async read(p: Uint8Array): Promise<number | null> {
for await (const ev of this.receive()) {
if (ev instanceof Uint8Array) {
const copiedFromBacklog = Math.min(
this.#readBacklog.byteLength,
p.byteLength,
);
p.set(this.#readBacklog.subarray(0, copiedFromBacklog));
this.#readBacklog = this.#readBacklog.subarray(copiedFromBacklog);
if (copiedFromBacklog === p.byteLength) {
this.#readBacklog = concat(this.#readBacklog, ev);
return copiedFromBacklog;
}
const diff = p.byteLength - copiedFromBacklog;
const copy = Math.min(diff, ev.byteLength);
p.set(ev.subarray(0, copy), copiedFromBacklog);
if (copy < ev.byteLength) {
this.#readBacklog = concat(this.#readBacklog, ev.subarray(copy));
}
return copiedFromBacklog + copy;
}
}
if (this.#readBacklog.byteLength) {
const copiedFromBacklog = copyBytes(this.#readBacklog, p);
this.#readBacklog = this.#readBacklog.subarray(copiedFromBacklog);
return copiedFromBacklog;
}
return null;
}
@keroxp, the purpose was to only handle data frames / messages. |
@keroxp Negligence on my part. Now that you raise the point, I agree that Reader/Writer isn't the right abstraction for a frame oriented protocol. Let's revert it. |
Resolves #5001