oven-sh / bun

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

Better API for WebSocket pub/sub #4353

Open codehz opened 10 months ago

codehz commented 10 months ago

What is the problem this feature would solve?

Currently, we can use 'server.publish' to publish message, but there is no way to do this if using export default {} style server (since you cannot get the server instance in this mode)

Even using "traditional" Bun.serve method to run the server, you can only get the server instance after invoking the Bun.serve, so some business logic that relay on pub/sub need to be adjusted for the api (e.g. you cannot init your class before bun.serve, or have to using later binding tricks, which is ugly)

What is the feature you are proposing to solve the problem?

My proposal for this problem is: add a PubSubController class, which can be initialised independently with Bun.serve (or export default {} style), and let it to be a option to the Bun.serve api, e.g.

const pubsub = new PubSubController();

// now you should able to use pubsub.publish() in any code
const my_business_logic = new MyBusinessLogic(pubsub);

export default {
  fetch() {...}
  websocket: {
    pubsub_controller: pubsub,
    ...
  }
}

This class does not need to implement the real publish api, it just forwards all messages to the native http server for implementation.

What alternatives have you considered?

current workaround:

const service = new MyServiceImpl({
  publish(topic: string, data: string) {
    // "circular dependency"? which is bad
    server.publish(topic, data);
  }
});
const server = Bun.serve(service);
evanwashere commented 10 months ago

you can access server object through this

export default {
  fetch(req) {
    if (this.upgrade(req)) return;
    if ('POST' === req.method) this.publish('topic', 'hello fellow websockets');

    return new Response('');
  },

  websocket: {
    message() {},

    open(socket) {
      socket.subscribe('topic');
    },
  },
}
codehz commented 10 months ago

through this

(it also available through fetch's 2nd parameter)

However, I still don't think it is an ideal API. You can only get the server instance after receiving the first request. (and still need using later binding trick to build abstraction)

Sure, publishing is meaningless before the first request is received, but it seems like an arbitrary limit and doesn't provide any performance benefit.

evanwashere commented 10 months ago

However, I still don't think it is an ideal API. You can only get the server instance after receiving the first request. (and still need using later binding trick to build abstraction)

Sure, publishing is meaningless before the first request is received, but it seems like an arbitrary limit and doesn't provide any performance benefit.

It's deliberately designed this way so bad/wrong abstractions are harder to make and to make it fit serverless lifecycle

instead abstraction should yield control to the user of Bun.serve(...) with lifecycle hooks for cron/open/close/fetch/message/upgrade

const abstraction = new ...;

export default {
  cron(ctx) {
    if (ctx.name === 'time_to_publish') {
      abstraction.publish_something(this);
    }
  },

  fetch(req) {
    abstraction.publish_something(this);
  },

  websocket: {
    message(socket) {
      abstraction.publish_something(socket);
    },
  },
}

You should only ask for server binding when publishing without user trigger/interaction (in serverless setTimeout/setInterval will likely be heavily limited unlike cron default export)

const server = Bun.serve({ ... });

const publish_timer = abstraction.create_publish_timer(server); // abstraction for setInterval(...)
publish_timer.stop();

one QOL change we could do is not error when Bun.serve(...) is default export

making this possible

const abstraction = {
  bind(server) {
    return this.server = server;
  },

  serve(...args) {
    return this.server = Bun.serve(...args);
  },
};

export default abstraction.serve(...);
export default abstraction.bind(Bun.serve(...));
codehz commented 10 months ago

serverless lifecycle

Tons of bun/node api "should" not be used in serverless mode, and I think bun is not specially designed for serverless usage.

If some API doesn't make sense in serverless mode, it can be disabled only in serverless mode, instead of not providing at all.

A normal server mode is equally important as serverless mode. pubsub is a useful feature, it should not be limited in user interaction scenario.

The publish messages can be sent from external event source like real-time temperature sensor value, real-time stock price which has nothing to do with user interaction. (it could be done by cron and pulling, but not every event source like it, some IoT device use SSE api to get real-time information)

evanwashere commented 10 months ago

A normal server mode is equally important as serverless mode. pubsub is a useful feature, it should not be limited in user interaction scenario.

The publish messages can be sent from external event source like real-time temperature sensor value, real-time stock price which has nothing to do with user interaction.

that's where these examples and proposed QOL change come in

const server = Bun.serve(...)
// works on both server and bun's serverless

You should only ask for server binding when publishing without user trigger/interaction

const server = Bun.serve({ ... });

const publish_timer = abstraction.create_publish_timer(server); // abstraction for setInterval(...)
publish_timer.stop();

one QOL change we could do is not error when Bun.serve(...) is default export

making this possible

const abstraction = {
  bind(server) {
    return this.server = server;
  },

  serve(...args) {
    return this.server = Bun.serve(...args);
  },
};

export default abstraction.serve(...);
export default abstraction.bind(Bun.serve(...));

Tons of bun/node api "should" not be used in serverless mode, and I think bun is not specially designed for serverless usage.

Bun.serve()/Bun.listen()/export default {} were designed specifically so that zig owns all io/net/crypto/timers and javascript is only responsible for "business" code. this allows for js side of runtime to be put in hibernation or even fully killed at any time without affecting currently active connections.

You can already see this in effect with bun --hot and it might trickle down to other features like bun --smol to further benefit both server and serverless users

and to be clear this brings major benefits for bun's serverless platform, other traditional serverless platforms don't take advantage of such design but will still see quite big speed improvement compared to node's builtin libraries.