QUIC client breaks inside electron

[lukasrakauskas]

Describe the bug

Electron has dropped support for external buffers. Thus some methods that this quic library uses are not working in such environment.

Error: Create external arraybuffer failed
    at get connectionId (/Users/lukas.rakauskas/dev/electron-js-quic-reproduce/node_modules/@matrixai/quic/dist/QUICConnection.js:326:36)
    at QUICClient.createQUICClient (/Users/lukas.rakauskas/dev/electron-js-quic-reproduce/node_modules/@matrixai/quic/dist/QUICClient.js:148:45)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  code: 'Unknown'
}

To Reproduce

I've made a reproduce repo. To reproduce pull it run npm install and npm run start
https://github.com/lukasrakauskas/electron-js-quic-reproduce

Expected behavior

QUIC client should be able to connect to a server, read and write to streams when created within electron.

Platform

• Device: Macbook M1 Pro
• OS: macOS Sonoma 14.5

Possible fix

The failing point is reading source ID in javascript, converting Uint8array to external vec seems to solve the external array buffer problem.

// src/native/napi/connection.rs

#[napi]
pub fn source_id(&self) -> External<Vec<u8>> {
  let source_id = self.0.source_id();
  let bytes = source_id.as_ref().to_vec();
  External::new(bytes) 
}

After this, the js fails to read header dcid property that also is Uint8array. To solve this, I've added get_dcid method to Header struct to be used instead of reading dcid directly.

// src/native/napi/packet.rs

#[napi]
pub fn get_dcid(&self) -> External<Vec<u8>> {
  External::new(self.dcid.to_vec()) 
}

Finally the library started to work for my use case

[linear]

ENG-368 QUIC client breaks inside electron

[CMCDragonkai]

Thanks for reporting this @lukasrakauskas. I'm going to be honest with you here, our company doesn't have the bandwidth to fix this atm, we're focusing on other aspects of our product development that does not involve electron atm. If you can send over a PR, we can validate it works and accept.

[ilkkapoutanen-61n]

Just hit this same roadblock myself. @lukasrakauskas I'd really appreciate that PR 🙏

[lane-eden]

hi, we are hitting this as well. @lukasrakauskas did you ever get a working PR?

[tegefaulkes]

@aryanjassal since you're digging around in js-quic right now, you could take a look into this. If it's simple the go ahead and fix it.

For reference https://github.com/nodejs/node-addon-api/blob/main/doc/array_buffer.md directs to https://github.com/nodejs/node-addon-api/blob/main/doc/external_buffer.md which gives some details relating to it. Defining NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED should reveal where the problems are and using Napi::Buffer::NewOrCopy may fix it. You may need something different for the UInt8Array type.

The use of External::new(bytes) isn't appropriate since I think it just avoids the problem by keeping the memory in the native space thus removing the ability to read it in the JS space. We need to be able to read the Ids in the js space.

[aryanjassal]

I did some digging into it by first attempting to replicate the issue using the provided repro code (which was really helpful btw). Once I could see the issue as it was happening, I tried to update js-quic to the latest version. The latest version is now ESM, so I had to make some minor changes to the code to make it accept the latest version. I could see that the bug was still present, so I started digging into potential fixes.

Nix note: electron-forge is a dynamically-linked executable, so I just used steam-run to launch it.

Then, I linked js-quic locally so I can test changes quickly. In my testing, I tried to use napi::bindgen_prelude::Uint8Array. However, electron wasn't happy with this either. Then, I tried to include the node env and let V8 create and manage memory directly. This seemed to work, but I was unable to re-cast the memory back into Uint8Array.

// v1
#[napi]
pub fn source_id(&self, env: Env) -> napi::Result<napi::JsBuffer> {
  let src = self.0.source_id();
  let mut js_buffer = env.create_buffer(src.len())?;
  js_buffer.as_mut().copy_from_slice(src.as_ref());
  Ok(js_buffer.into_raw())
}

// v2
#[napi]
pub fn source_id(&self, env: Env) -> napi::Result<napi::JsTypedArray> {
  let src = self.0.source_id();
  let array_buffer = env.create_arraybuffer(src.len())?;
  let typed_array = array_buffer.into_raw().into_typedarray(TypedArrayType::Uint8, src.len(), 0)?;
  Ok(typed_array)
}

Both approaches worked well with electron and didn't need any JS-layer changes, but that was because only basic attributes were used, like byteLength and byteOffset, which existed on both JsTypedArray and JsBuffer. To make it actually compatible with other, non-NAPI code, the return value needs to be a Uint8Array, as that directly corresponds to vec<u8> type in rust.

So, while this is a workaround patch, it is not yet ready to be merged into the main repo. This will need more research.

---

For reference https://github.com/nodejs/node-addon-api/blob/main/doc/array_buffer.md directs to https://github.com/nodejs/node-addon-api/blob/main/doc/external_buffer.md which gives some details relating to it. Defining NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED should reveal where the problems are and using Napi::Buffer::NewOrCopy may fix it. You may need something different for the UInt8Array type.

This is not as simple, unfortunately. I could not find a way to define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED because rust does not support C-style preprocessor defines. Technically, a single reference exists in the napi-rs code for this string,
but I could not find a way to enable it manually. https://docs.rs/napi/latest/napi/enum.Status.html#variant.NoExternalBuffersAllowed

Skimming through the source code for napi-rs also did not show me any obvious way to enable this flag, so I could not rely on this define to trace where this is happening.

[bergmorten]

I've also hit this error/limitation :-(