account for RangeError: Array buffer allocation failed

[jackyzha0]

Why

function replicateArrayBufferError() {
  try {
    const size = 2 * 1024 * 1024 * 1024;
    const buf = new ArrayBuffer(size);
	conn.send(buf);
  } catch (e) {
    console.error(e);
  }
}

replicateArrayBufferError();
console.log('you can try catch this!!!')

tldr; a failed allocation can be try-caught by a top-level handler (e.g. in react) and river can pretend nothing went wrong and construct another message

by the time the next message comes around, its possible theres enough memory to allocate again and whoops!!! we skipped sending a message

some more context on this error: https://groups.google.com/a/chromium.org/g/blink-dev/c/pZ9kld0LehA/m/Ho-SCn8tBAAJ

What changed

• assume codecs are fallible: wrap codec in an adapter (CodecMessageAdapter) to try catch toBuffer and fromBuffer operations
• delete the session if codec fails, this should gc the message buffer for that session which should help memory pressure
• we also expose ProtocolError.MessageSendFailure so the consumer can decide to shut down the world in this case too
• modify ws connection to not throw when sending on CLOSING or CLOSED states (safe to discard as we have the sendbuffer)
• actually read the result of .send()
• added test to prevent regression
• delete unused messageFraming code (we axed UDS a while ago)

Versioning

• Breaking protocol change
• Breaking ts/js API change

[masad-frost]

I'm not sure how i feel about absolutely swallowing everything here. Should we log the error?

[jackyzha0]

this only ever fails for one reason, sending on CONNECTING state, imo its fine? connection doesnt have direct access to a logger and i think threading it through just for this doesnt super make sense

we can alternatively trigger this.ws.onerror and close

[masad-frost]

we should leave doc strings on all of our classes!

[masad-frost]

curious why an adapter instead of making the implementation conform to this

[jackyzha0]

codec is user substitutable and we cant expect every consumer to remember to try/catch toBuffer fromBuffer, so we do it at this layer

[masad-frost]

what's the significance of moving this up? maybe worth a comment

[jackyzha0]

this is the point where we 'consume' the pending session and turn it into a connected session

now that there is a fail point, we want to clear the old pending session earlier rather than later

[masad-frost]

FIVE BIG BOOMS

💥
💥
💥
💥
💥

tiny comments