Ability to check whether data is available or a read timeout

[marciot]

Right now, when you call await serial.readable.getReader().read(), it will block indefinitely if no data is available. This works alright when you know you are expecting data, but it won't work very well if you do not know this for certain or when serial errors cause data to be dropped.

It is possible to implement a read timeout using setTimeout, but I found it is somewhat tricky to get right and I had to go through several iterations to figure out how to make it work:

async read(timeout) {
    let timeoutId;
    let timeoutPromise = new Promise(
            (resolve, reject) =>
                this.timeoutId = setTimeout(
                    () => reject(new SerialTimeout("Timeout expired while waiting for data")),
                    timeout
                )
    );
    if(!this.readPromise) {
        this.readPromise = this.reader.read();
    }
    const result = await Promise.race([this.readPromise, timeoutPromise]);
    this.readPromise = null;
    clearTimeout(this.timeoutId);
    return result;
}

Having a timeout helps in some situations, but it really seems like a clumsy work around to not being able to check whether data is available before calling serial.readable.getReader().read()

[reillyeon]

This is mostly a feature request for the ReadableStream API so adding @domenic and @ricea for visibility.

In WritableStreamDefaultWriter there is a desiredSize property and I could imagine a similar availableSize attribute being added to ReadableStreamDefaultReader. Note that for the Web Serial API, at least as implemented in Chromium, the desiredSize attribute doesn't work correctly for reasons explained in the specification. We would need similar hooks between the ReadableStream and UnderlyingSource to allow the source to specify how much data it could potentially return from pull() without actually enqueuing it.

[domenic]

Hmmm. I'm unsure exactly what problem you're trying to solve here:

1. Is it reading with a timeout? I can certainly imagine making that more ergonomic in various ways, and if you tell me this is what you're trying to solve, I am happy to go into more detail on potential improvements there.

2. Is it checking synchronously whether data is available? I'm having a harder time imagining how this would work. Do you poll such a property on a setInterval()? That seems pretty strange, and backward from how we do things in JavaScript typically. I.e. in JS we use actual asynchronous APIs, instead of sync APIs + polling.

[marciot]

The place where this come into play is 3D printing. For every command you send to a 3D printer, you should expect a reply, this leads to code that looks something like this:

async printLoop(commands) {
   while(commands.length) {
      await sendCommand(commands.pop());
      await getReply();
   }
}

However, for various reasons, such as bugs in the firmware, or slight mismatch in the serial data rates between the computer and the microcontroller used in the printers, occasionally a reply is never received. This leads to a stall in the print, unless the code is modified as such:

async printLoop(commands) {
   while(commands.length) {
      let pendingCommand = commands.shift();
      await sendCommand(pendingCommand);
      try {
         await getReplyWithTimeout(10000);
      } catch(e) {
         if(e instanceof SerialTimeoutError) {
            // Resend the command
            command.unshift(pendingCommand);
            continue;
         } else {
             throw e;
         }
      }
   }
}

As for the second question, I didn't have a specific use case in mind, except to think that it could possibly be used to solve the problem presented in the first situation. It's possible that indeed it isn't necessary, if a timeout facility was provided.

[reillyeon]

To handle timeouts whatwg/streams#1103 is probably what you need. That assumes, however, that you actually want to cancel the call to read(). This isn't strictly necessary. Some piece of the code which is listening for messages can be kept active at all times waiting for it to respond. As written in the original example there's actually a problem because since the read is never cancelled the next chunk, when it arrives, will be discarded.

I'm curious if developer using WebSocketStreams are running into similar issues. Ignoring HTTP streaming, for a fetch-based stream you don't find yourself handling discrete chunks of the data over a long time-scale the way you do for a long-lasting connection.

[domenic]

OK, cool. Yeah, so my personal view of the ideal solution here would be:

• Make read requests easily abortable using an AbortSignal: Make read requests abortable whatwg/streams#1103

• Make it easy to generate AbortSignals which abort after a timeout: Add AbortSignal.timeout(ms) whatwg/dom#951

Then you would write getReplyWithTimeout(10_000) as

try {
  const { value, done } = await serialReader.read({ signal: AbortSignal.timeout(10_000) });
} catch (e) {
  if (e.name === "AbortError") {
    // TODO
  } else {
    throw e;
  }
}

[marciot]

To handle timeouts whatwg/streams#1103 is probably what you need. That assumes, however, that you actually want to cancel the call to read(). This isn't strictly necessary. Some piece of the code which is listening for messages can be kept active at all times waiting for it to respond. As written in the original example there's actually a problem because since the read is never cancelled the next chunk, when it arrives, will be discarded.

Actually, I believe this criticism applies to my first implementation, which looked like this:

async read(timeout) {
    let timeoutId;
    let timeoutPromise = new Promise(
            (resolve, reject) =>
                this.timeoutId = setTimeout(
                    () => reject(new SerialTimeout("Timeout expired while waiting for data")),
                    timeout
                )
    );
    const result = await Promise.race([this.reader.read(), timeoutPromise]);
    clearTimeout(this.timeoutId);
    return result;
}

This version is actually broken in precisely the way you explained. This is why I said writing the code correctly is non-trivial. I fixed it by saving the promise object for reuse in the next call to "read()".

[marciot]

OK, cool. Yeah, so my personal view of the ideal solution here would be:

• Make read requests easily abortable using an AbortSignal: whatwg/streams#1103
• Make it easy to generate AbortSignals which abort after a timeout: whatwg/dom#951

Then you would write getReplyWithTimeout(10_000) as

try {
  const { value, done } = await serialReader.read({ signal: AbortSignal.timeout(10_000) });
} catch (e) {
  if (e.name === "AbortError") {
    // TODO
  } else {
    throw e;
  }
}

@domenic: This would work for me. The syntax is a bit strange, I would have expected const { value, done } = await serialReader.read({timeout:10_000});, but I am not familiar with signals so it's just probably my unfamiliarity with them.

Anyhow, it seems like my request is a duplicate of that other ticket. Glad it's being looked at. Thank you for your feedback!

[domenic]

In whatwg/streams#1103 folks are pointing out that it's very rare that you want to cancel a read() call without canceling other subsequent read() calls. At first I thought this thread was a counterexample, but now I am not so sure. In particular, @reillyeon's point

That assumes, however, that you actually want to cancel the call to read(). This isn't strictly necessary. Some piece of the code which is listening for messages can be kept active at all times waiting for it to respond.

indicates a different strategy, which is more like

async printLoop(commands) {
   while(commands.length) {
      let pendingCommand = commands.shift();
      await sendCommand(pendingCommand);

      let gotReply = false;
      setTimeout(() => {
        if (!gotReply) {
          // Resend the command
          sendCommand(pendingCommand);
        }
      }, 10_000);

      await getReply(); // no timeout
      gotReply = true;
   }
}

although that only handles a single retry, hmm...

[marciot]

@domenic: This is a very simplified example. The actual code for interacting with the printer is much more complicated that this. Of the top of my head, simply resending a command like this would probably not work.

Also, I believe that the idea behind async/await is to allow people to write code with a more familiar synchronous paradigm. Adding a timeout like that breaks that abstraction in a way that concerns me a lot.

[marciot]

Here is an example that still is simplified, but gets closer to what actually needs to happen:

async printLoop(commands) {
   while(commands.length) {
      let pendingCommand = commands.shift();
      await sendCommand(pendingCommand);
      try {
         await getReplyWithTimeout(10000);
      } catch(e) {
         if(e instanceof SerialTimeoutError) {
            // Resend the command later
            command.unshift(pendingCommand);
            // Put printer back into a known state
            await sendResetCommandToPrinter();
            // Purge the input buffers
            while(serial.dataAvailable) {
                serial.read();
            }
            continue;
         } else {
             throw e;
         }
      }
   }
}

Purging the data on the port is one situation where checking whether bytes are available could help; doing so with a read() with timeout introduces a slight delay, which isn't a problem in this case.

[domenic]

The point we're trying to make is that you should be able to completely avoid the read-with-timeout pattern, and instead have a concurrent bit of code running which notices that a response hasn't arrived yet, and performs any necessary resets at that time.

That way, you maintain the invariant that a single read request always results in a single result, instead of having to issue potentially N read requests, the first N - 1 of which timeout.

[reillyeon]

Purging data from the port can be accomplished by calling abort() on the ReadableStream (or reader). This will return a Promise and close the stream. Once the Promise resolves you can access the readable attribute on the SerialPort to get a new ReadableStream.

[marciot]

The point we're trying to make is that you should be able to completely avoid the read-with-timeout pattern, and instead have a concurrent bit of code running which notices that a response hasn't arrived yet, and performs any necessary resets at that time.

That way, you maintain the invariant that a single read request always results in a single result, instead of having to issue potentially N read requests, the first N - 1 of which timeout.

Hummm. I see what you are saying. I'm not arguing that it can't be done that way. But I guess I will counter that at least a certain number of people will be porting serial code that was written in a sequential manner. A lot of serial port code is written that way and will rely on the timeout pattern; forcing that code to be rewritten as concurrent code could present significant problems.