Open karelv opened 11 months ago
Note: I'm hinting it with using interval=10 (10 ms) that it occurs faster with more data, increasing that interval to 100ms would require 10x more time or ~300 seconds before the failure is active, similar longer time (I didn't time accurately) is observed with 1 sec interval.
Using https://github.com/googlechromelabs/serial-terminal/ (or the website) I found that it results in a couple minutes to crashing page: Now, my thinking it is likely related to the fact that the terminal(xterm) on the webpage gets too full or loaded, therefore I forked that repo, and I cleared the terminal every second. https://github.com/GoogleChromeLabs/serial-terminal/compare/main...karelv:serial-terminal:main I don't intend to PR it, as it is pretty annoying for use, but now I don't get the above 'Snap' page, and instead I got the same error as I previously observed with my own html/javascript implementation. As you can see in less than 3 minutes the error appears.
Using Samsung Galaxy S21 5G (SM-G991U)
Hi, I also had this error in my application (sending and receiving a firmware image of about 50 kB).
It seems UsbEndpointUnderlyingSource.pull() in the polyfill is to blame. I made it a proper async function, returning the Promise, not just enqueing its execution, and that solved it for me. See the attached patch. This seems to be alright according to the API (https://streams.spec.whatwg.org/#underlying-source-api, kindly referenced in serial.ts).
Without this change, multiple parallell calls to transferIn were made. I got errors when somewhere above 12000 calls were on-going.
At one point I also got an error when ReadableByteStreamController.enqueue() was called with a zero-length chunk, but now I fail to reproduce it. I added a check for chunk's length.
I was not able to test this using karelv's program. Seems like I need an USB-OTG adapter, as my Arduino board has a type B port.
(I'm using a Lenovo tablet TB310FU with Android 13 and Chrome 128)
Thank you @klaarv for that investigation. This is strange behavior as the Streams API specification says the pull()
method will not be called repeatedly unless it enqueues a chunk. Since that only happens when the transferIn()
call completes I don't understand what could be happening here.
@ricea, has there been a change to the behavior of the Streams API? Lots of transferIn()
calls would explain the behavior seen here.
@reillyeon Yes, the specification says so. And on the next line: "If the function returns a promise ...".
But does it also say that pull()
is of type UnderlyingSourcePullCallback
which returns a Promise<undefined>
?
There hasn't been a change to the spec, but I can't rule out a bug in the implementation.
Here is a program which I think reproduces our situation, with multiple awaits run in parallell.
It creates and reads a readable stream, which produces a few chunks at a few different points in time.
pull()
is called twice for each read, before and after. I have run it in Firefox, Chrome and Node with similar results.
There are several ways to limit the number of parallell awaits:
pull()
.pull()
.start()
instead of in pull()
.highWaterMark
to 0.But this is mostly a way for me to learn a little about the streams api. I guess modifying pull()
in one way or the other is the most appropriate.
const start = Date.now();
function getTime() {
return Date.now() - start;
}
class MyUnderlyingByteSource implements UnderlyingByteSource {
type : 'bytes' = 'bytes';
count = 0;
waiting = 0;
work : Promise<Uint8Array>[];
constructor() {
this.work = [];
for (let i = 0; i < 10; i++) {
this.work.push(new Promise(resolve => {
setTimeout(() => {
resolve(new Uint8Array(10));
}, 500*(i+1));
}));
}
}
async awaitAndEnqueue(controller : ReadableByteStreamController) : Promise<void> {
const c = this.count++;
if (c < this.work.length) {
this.waiting++;
console.log(`${getTime()}: awaits chunk ${c}, ${this.waiting} waiting`);
const chunk = await this.work[c]
this.waiting--;
console.log(`${getTime()}: enqueues chunk ${c}`);
controller.enqueue(chunk);
}
else {
console.log(`${getTime()}: would await chunk ${c}`);
}
}
//start(controller : ReadableByteStreamController) {
// (async () => {
// while (this.count < this.work.length) {
// await this.awaitAndEnqueue(controller);
// }
// })();
//}
pull(controller : ReadableByteStreamController) : void {
//if (this.waiting < 2) {
this.awaitAndEnqueue(controller);
//}
}
//async pull(controller : ReadableByteStreamController) : Promise<void> {
// await this.awaitAndEnqueue(controller);
//}
}
const readable = new ReadableStream<Uint8Array>(
new MyUnderlyingByteSource(),
{ highWaterMark: 20 }
);
(async () => {
for await (const bytes of readable) {
console.log(`${getTime()}: Read ${bytes.length} bytes`);
//await new Promise(resolve => setTimeout(resolve, 550));
}
})();
As pointed in #57, the web-page faces an issue after X amount of time (or data, or events) has been passed. The message is:
NetworkError: Failed to execute 'transferIn' on 'USBDevice': A transfer error has occurred.
In order to reproduce the error I made a small web-page and a small Arduino sketch. Both can found here:
https://github.com/karelv/web-serial-example
And here: https://karelv.github.io/web-serial-example/
Steps to reproduce:
interval=10
+ click sent buttonchunk=9999
+ click sent buttonAfter I got:
And chrome://device-log gives:
Compared to the observation in #57 there are only 2 error message (and 2 debug message) whereas in #57 they were countless. Therefore it is wise to review the javascript code I am using that webpage, maybe there is something wrong on my end.