Hooks

[bminer]

Hooks

[bminer]

Understood. Thanks so much for your feedback, @daniandl. I'll think this over a bit more, and I'll post back here if/when the change is made.

[bminer]

I created a PR for this (see above). The code for this has not been tested at all. Any feedback (or bug fixes!) would be appreciated!

[bminer]

I'd love to hear more about your use case. Here are some of my thoughts:

• Rate Limiting could be implemented by listening on the message event handler and simply counting the number of messages received by a socket (as I mention here). If the count is too high or if the message itself gets too large, you can emit an event to tell the client to slow down a bit. If the client doesn't listen, you can simply drop the connection, ban/throttle connections based on IP, etc.
• Authentication should be done when the socket connects for the first time. After that, it should not be necessary, so IMHO auth middleware would not really be appropriate. Each socket already has a get and set function to allow you to associate data with the socket itself (i.e. the logged in user).
• Other Middleware could be implemented by this lib, but I can't help but wonder... what is the use case for it?

[daniandl]

Well one may want to return an error instead of banning or terminating a connection. For example in my case, using my "internal hooks workaround" i was able to implement rate limits for each endpoint (socket event) and also a global one. If the user goes above the limit, i return an error to his request id.

About authentication, i agree with your point but auth doesn't necessarily end at login. You could for example write a decorator to check for specific permissions or user role. This way you keep your code dry and have less if(x) throw at the top of your app. (Edit: i meant authorization, my bad)

You could also build a namespace system and use the onReceive hook to check if the user is part of said namespace. Have a look at Fastify's decorators (im sure express has something similar).

An example for a potential onSend hook, not just onReveice like above:
You may also want to enforce a specific json structure when a response is successful like wrapping the data in a response object. You may also want to pass all your data through a serializer before sending it to a client, the possibilities are endless :D

Happy new year 🎉

[bminer]

I think that I understand. Thank you for clarifying your use case!

I can definitely understand grouping many event handlers together into a group where you could use a single hook to check for permissions. This would definitely be better than checking permissions in each individual event handler. I'll think this over and see if there is currently a way to do this using the "message" event handler. Honestly, I doubt it, and this is a strong case for hooks to be in the lib.

If you don't hear from me in a few days, please feel free to ping / nudge me.

Happy New Year to you, as well!

[bminer]

@daniandl - So how do you feel about wrapping the event handlers?

Instead of:

socket.of("subsystem").on("event1", function (e) {
	if(this.get("reqCount") > REQUESTS_PER_MIN) {
		throw new Error("Exceeded request limit of " + REQUESTS_PER_MIN + " requests per minute")
	}
	console.log("Event 1 occurred:", e)
}).on("event2", function (e) {
	if(this.get("reqCount") > REQUESTS_PER_MIN) {
		throw new Error("Exceeded request limit of " + REQUESTS_PER_MIN + " requests per minute")
	}
	console.log("Event 2 occurred:", e)
})

You would have:

socket.of("subsystem").on("event1", reqLimit(function (e) {
	console.log("Event 1 occurred:", e)
})).on("event2", reqLimit(function (e) {
	console.log("Event 2 occurred:", e)
}))

function reqLimit(handler) {
	return function(e) {
		if(this.get("reqCount") > REQUESTS_PER_MIN) {
			throw new Error("Exceeded request limit of " + REQUESTS_PER_MIN + " requests per minute")
		}
		return handler(e)
	}
}

You could do something similar for all types of middleware, including authorization, etc. Thoughts?

[daniandl]

That is indeed a very good workaround for an onReceive hook! However this does require you to import said handler into each file where socket events are being listened on. Same would go for onSend where you could just pass everything through a function before emitting/returning.

My current backend has a registerEndpoint function where you can pass an event name, a function to call and then options. The options include rate limits per endpoint and also an array of functions that should run (resolve) before the main one is called.

WebSocketServer.registerEndpoint('chat.message', onChatMessage, {
  limit: 3,
  duration: 3,
  namespace: 'chat',
  decorators: [isAuthed, hasPerm('CAN_CHAT')],
});

This is obviously a very specific issue related to my use cases but I still personally think that hooks would do more good than harm to the lib :)

I would totally understand if you say that a task like this is better left up to the person implementing the wrapper rather than adding to bundle size (since the wrapper is also used in frontend)

[bminer]

I'm still on the fence here, but I have a proposal:

We could add a use method to the WebSocketChannel that provides middleware support to process inbound messages. Middleware could therefore be scoped to a given channel or be used on the "default / root" channel. The signature is as follows:

• channel.use(function middleware(eventName, args, next) {...})
  • eventName is the name of the received event
  • args is the array of arguments to be passed to the event handler
  • next should be called if processing should continue to the next middleware function

The middleware function can throw an Error. If the inbound message is a request, the error is returned to the remote end.
The middleware function can call next, which simply passes the message to the next middleware function. The last middleware function would then call the event handler if it exists.

I don't understand the use case for adding an onSend hook.

What are your thoughts?

[daniandl]

That sounds pretty promising! I would enjoy a feature like this being natively implemented.

I understand if you're on the fence about it though. Like said, I've solved this problem for me locally by overwriting the onMessage handler at runtime and was able to build a middleware system on top of it. So I'm not eagerly waiting on this to be natively implemented, it's rather just a suggestion or discussion on how to further improve the wrapper :)