Repository navigation
websockets: buffered amount not tracked in ArrayBuffer for typed arrays - fixes #5715 - #5716
Conversation
…ys or DataView fixes grafana#5715
|
|
ankur22
left a comment
ankur22
left a comment
There was a problem hiding this comment.
Thanks for the fix. I left some comments around what I think would improve the testing of the fix.
| ws.send(sent.subarray(0, 1)) | ||
| ws.send(sent.subarray(0, 10)) | ||
| if (ws.bufferedAmount != 10) { | ||
| throw "Expected 10 bufferedAmount got "+ ws.bufferedAmount |
There was a problem hiding this comment.
Is there a reason why you're throwing a string instead of a Error?
| require.Len(t, logs, 0) | ||
| } | ||
|
|
||
| func TestBinaryType_ArrayBuffer_issue_5226(t *testing.T) { |
There was a problem hiding this comment.
Is there a better place that modifying this test? Maybe a new test would be better?
Also need a test for DataView.
There was a problem hiding this comment.
Fixed, augmented the existing test TestArrayBufferViewSupport to have a separate pass for testArrayBufferViewBufferedAmount. I reused the original runner as it maintains a list of TypedArrays supported by Sobek.
Regarding DataView, I found that it isn't supported by this package. Trying to send a DataView invokes this part of the code resulting in an error:
case map[string]any:
rt := w.vu.Runtime()
obj := msg.ToObject(rt)
if !isBlob(obj, w.blobConstructor) {
common.Throw(rt, fmt.Errorf("unsupported send type %T", o))
}There was a problem hiding this comment.
Ok, can you update the issue and PR title/description by removing DataView? Thanks!
prakharbirla-ng
changed the title
| } | ||
| ws.onmessage = async (e) => { | ||
| const received = new %[1]s(await e.data.arrayBuffer()); | ||
| if (ws.bufferedAmount != 0) { |
There was a problem hiding this comment.
received is assigned but never used, can it be removed?
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
websockets: buffered amount not tracked in ArrayBuffer for typed arrays - fixes #5715 Co-authored-by: Iván Szkiba <szkiba@users.noreply.github.com>
What?
When sending TypedArrays, bufferedAmount was not incremented.
Fixes the bug introduced in #5358
Why?
Buffered amount should be tracked when data is added to the queue so that it does not become negative.
Checklist
make check) and all pass.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)
Closes #5715