Adapter life-cycle management and long-running process functionality

[bytenik]

You want to:

• [ ] report a bug
• [x] request a feature

Current behaviour

• socket.io instantiates an adapter, but does not wait for a callback that it is ready. This works fine for many scenarios, but not one where the adapter needs to spool up long-running processes where the adapter is not ready to do work until they are completed. Connections from clients are handled immediately, and if you use a simple pattern of emit-on-connect from client, an early client can easily connect prior to adapter readiness.
• socket.io calls functions on the adapter, and then immediately invokes its own internal callback function with an argument of null.
• socket.io does not notify its adapter that it is shutting down (closing). As a result, there is no opportunity to stop long-running processes within the socket or disconnect from whatever scale-out solution is being used.

Expected behaviour

• socket.io should pass a callback to the adapter constructor and wait for it to be called prior to continuing to stand up.
• socket.io should pass a callback function to adapter functions and then use that to trigger callback to the rest of the socket.io stack.
• socket.io should call a method on the adapter to indicate shutdown. It need not wait for a callback, because if shutdown fails, what are you going to do -- not shut down? The in-memory adapter can simply make this function a no-op. While I haven't reviewed the Redis adapter in enough depth to see if there is utility to this function in that adapter, I would imagine that it would be good to detach from Redis to return system resources.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

This is particularly critical for the adapter that I have built that works with AWS SQS/SNS: https://github.com/thinkalpha/socket.io-sqs (socket.io-sqs on npm)

For example, actions like addAll can't be properly handled in time; e.g. if you addAll a socket to room xxx and then immediately .to(xxx).emit(...), your message doesn't necessarily end up back at that socket.

[darrachequesne]

The adapter actually had a callback in the previous versions (included in Socket.IO v2), see here.

We've removed it since it was not needed anymore by the Redis adapter, but it seems other adapters may benefit from this. We can add it back in the form of a Promise, would it suit your use case?

Edit: could you please merge the 3 other issues into this one please, it will be easier to follow :angel:

[bytenik]

Yes, making it promise-based would be ideal!

[bytenik]

Incorporating #3661, #3660, and #3659 into this issue.

[bytenik]

I've worked around some of these issues in a really hacky way by making a factory for the adapter class and then firing callbacks on arguments passed into the factory, but its definitely not ideal.

[darrachequesne]

@bytenik I've updated the adapter class (https://github.com/socketio/socket.io-adapter/commit/2e023bf2b651e543a34147fab19497fbdb8bdb72) and the socket#join() method (https://github.com/socketio/socket.io/commit/129c6417bd818bc8b4e1b831644323876e627c13).

I'm not sure about the public API for the Adapter#init() and Adapter#close() methods though. Should those methods be called by the end users directly?

const server = require("http").createServer();
const io = require("socket.io")(server);

await this.of("/").adapter.init();

server.listen(3000);

// and then...
server.close();
await this.of("/").adapter.close();

[bytenik]

@darrachequesne Awesome, that looks great.

Right now the Adapter constructor is passed in, not a constructed adapter. socket.io constructs the adapter. So, there's no good way for the end user to call init or close. The lifespan is controlled by the io instance.

[darrachequesne]

Yes you are right. Currently, there is one Adapter per namespace, and they are constructed when:

• a namespace is created, with io.of("/my-namespace")
• the adapter constructor is updated (for all existing namespaces) with io.adapter(CustomAdapter)

But we could change that behavior, though I'm not really sure about the implications.

[bytenik]

@darrachequesne I don't think that there's anything inherently wrong with socket.io controlling the adapter lifecycle. I just think that in that case, the adapter needs an opportunity to start long running processes (init) and shutdown long running processes (close). So, both could return promises and socket.io would call those functions and then block until they resolve.

[bytenik]

Hey, I just got back to trying to implement these changes on my end. Is init ever being called? I can't find a call to it, and my debug messages indicate it isn't being hit.

[darrachequesne]

You are right, the Adapter#init method is not currently called.

We could call it here:

https://github.com/socketio/socket.io/blob/1faa7e3aea1414ec814aa935021356e8ed2b054c/lib/namespace.ts#L79-L81

But we cannot await a constructor (the initAdapter method is called under the hood):

const io = new Server({
  adapter: myCustomAdapter
});

Is there any problem with manually calling init()?

const io = new Server({
  adapter: myCustomAdapter
});
await io.of("/").adapter.init();

I'm open to suggestions on this.