oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.52k stars 2.79k forks source link

Bun breaks OpenAI Realtime API Beta: Bun hijacks 'ws' module Websockets; fails to implement finishRequest callback #14345

Closed K-Mistele closed 1 month ago

K-Mistele commented 2 months ago

What version of Bun is running?

bun --revision 1.1.17+bb66bba1b

What platform is your computer?

Darwin 23.6.0 arm64 arm

What steps can reproduce the bug?

install the openai/openai-realtime-api-beta package. The package is designed to run both on the server and in the browser.

Run the following code with bun run voice.ts - it will hang at new RealtimeClient. If you add the argument to RealtimeClient to allow setting the API key in the browser (because the package tries to execute the client code as described below), then it hangs at await client.connnect().

import { RealtimeClient } from '@openai/realtime-api-beta'
import 'dotenv/config'
//console.log(`websocket:`, globalThis.WebSocket)
console.log(`Creating client...`)

console.log(`api key:`, process.env.OPENAI_API_KEY)
const client = new RealtimeClient({ apiKey: process.env.OPENAI_API_KEY, dangerouslyAllowAPIKeyInBrowser: true });
console.log(`client created`)

// Can set parameters ahead of connecting
client.updateSession({ instructions: 'You are a great, upbeat friend.' });
client.updateSession({ voice: 'alloy' });
//client.updateSession({ turn_detection: 'server_vad' });
client.updateSession({ input_audio_transcription: { model: 'whisper-1' } });

// Set up event handling
client.on('conversation.updated', ({ item, delta }) => {
    const items = client.conversation.getItems(); // can use this to render all items
    /* includes all changes to conversations, delta may be populated */
});

console.log(`connecting...`)
// Connect to Realtime API
await client.connect();
console.log(`Connected!`)

// Send a item and triggers a generation
client.sendUserMessageContent([{ type: 'input_text', text: `How are you?` }]);

What is the expected behavior?

Bun breaks a few things with this package:

  1. the package uses globalThis.WebSocket to detect if it should run the client websockets or server websockets with the ws module; which Bun implements even though it is not a browser. Probably, the package should be patched to use globalThis.window for browser detection. This is why it is necessary to set dangerouslyAllowAPIKeyInBrowser in the example above, since the code thinks it's browser code. While this isn't technically a bug in Bun, it's undefined behavior caused by Bun deviating from Node's API, and the failure modes are non-obvious for reasons described below.

If you patch the package in node_modules/@openai/realtime-api-beta/lib/api.js to use globalThis.window for browser detection, a couple of things happen:

  1. The package correctly executes the Node portion of code (starting at line 109) to set up the websocket (below code copy/pasted from the package). But, while the package tries to import WebSocket from the ws module, the type of the wsModule.default and of WebSocket are still BunWebSocket. Bun "hijacking" the ws module is definitely unexpected, and in this case, undesired behavior.

  2. The reason that the hang occurs, is that the package relies on the ws module's finishRequest method to finish adding the headers to the websocket request before it is self. Bun seems not to implement this. Running the code above using bun run voice.ts does not result in execution entering the finishRequest function below, or a console.log() line that I tried placing in it being executed. This results in the await client.connect() hang that's described above - because the client is set up, but when the connection request is sent without the proper authorization headers, it causes the connection to be closed with a bad_request_error since the Authorization header is missing.

const moduleName = 'ws';
      const wsModule = await import(/* webpackIgnore: true */ moduleName);
      const WebSocket = wsModule.default;
      const ws = new WebSocket(
        'wss://api.openai.com/v1/realtime?model=gpt-4o-realtime-preview-2024-10-01',
        [],
        {
          finishRequest: (request) => {
            // Auth
            request.setHeader('Authorization', `Bearer ${this.apiKey}`);
            request.setHeader('OpenAI-Beta', 'realtime=v1');
            request.end();
          },
        },
      );
      ws.on('message', (data) => {
        const message = JSON.parse(data.toString());
        this.receive(message.type, message);
      });
      return new Promise((resolve, reject) => {
        const connectionErrorHandler = () => {
          this.disconnect(ws);
          reject(new Error(`Could not connect to "${this.url}"`));
        };
        ws.on('error', connectionErrorHandler);
        ws.on('open', () => {
          this.log(`Connected to "${this.url}"`);
          ws.removeListener('error', connectionErrorHandler);
          ws.on('error', () => {
            this.disconnect(ws);
            this.log(`Error, disconnected from "${this.url}"`);
            this.dispatch('close', { error: true });
          });
          ws.on('close', () => {
            this.disconnect(ws);
            this.log(`Disconnected from "${this.url}"`);
            this.dispatch('close', { error: false });
          });
          this.ws = ws;
          resolve(true);
        });
      });
    }

What do you see instead?

  1. Prevent Bun from hijacking the ws module's WebSocket implementation & exports, when they are specifically requested.
  2. Implement finishRequest in Bun's WebSocket

Additional information

Twitter thread where I originally debugged this is here: https://x.com/0xblacklight/status/1842035646719811803

sirenkovladd commented 2 months ago

unfortunately Bun does not use the options parameter in WebSocket so any arguments like finishRequest or agent will not be used

I guess we can rename this ticket to something like Support the options parameter for ws.WebSocket

K-Mistele commented 2 months ago

also, it should probably not override the WebSocket in the ws module when it's being explicitly imported and used.

7heMech commented 1 month ago

also, it should probably not override the WebSocket in the ws module when it's being explicitly imported and used.

It's fine since Bun aims to have everything go faster when they provide same API as WS module it just makes all those packages way faster

K-Mistele commented 1 month ago

Is this released in v1.1.30? https://bun.sh/blog/bun-v1.1.30

NoRKin commented 1 month ago

Using bun v1.1.33, still hangs on client.connect()