onMessage hook?

[daniandl]

First, very nice wrapper! :)

I'm not sure but it seems to me there isn't a hook to catch all received messages.
This would be very useful server side, as it would make rate limiting a lot easier.

Do you have a suggestion on how to best implement this, so that I don't have to check on every socket.on?

The reason I ask here and not in ws-server-wrapper, is because ws-server-wrapper extends ws-wrapper (and therefore also the onMessage event).

[bminer]

This is a great question. I'll look into it...

[bminer]

@daniandl - Whenever a message is received, the "message" event is fired, as you can see here: https://github.com/bminer/ws-wrapper/blob/master/lib/wrapper.js#L87-L91

But the question remains: how do you implement rate limiting? This might be a case where ws-wrapper should provide the functionality out of the box...

I don't know. Do you have any other thoughts?

[bminer]

One idea... use the "message" event to count inbound messages per connection. If the message count (or total byte count) is too high for a given window, you just call disconnect() on the socket. You can get more sophisticated by keeping track of offending IP addresses in a database or whatever and randomly dropping these connections before HTTP upgrade.

Anyway, please feel free to post back here, but I'm going to close for now. Thanks!

[bminer]

Also with ws-server-wrapper, you can easily count the total number of incoming messages (not per socket) using server.on("message", listener). In this case listener will be called for each inbound message. Within the listener itself, this would refer to the wrapped socket, I believe.

[daniandl]

Hi @bminer, thanks for looking into this so quickly :)

Yes, I'm aware that it's possible to write rate limit logic to flag clients, but it's tedious to then have to check if(isRateLimited) on every socket.on server-side :D

I spent some time today making my own implementation, and I've found a way to make it work without needing to fork this repo, nor ws-server-wrapper:

onConnection

const oldHandler = socket._onMessage
socket._onMessage = newHandler.bind(socket, oldHandler)

newHandler(function)

function newHandler(oldHandler, data) {
  const socket = this  // quality of life

  // i apply "rateLimited" to the socket in my rate limiting system, not relevant here
  if (socket.get('rateLimited')) {
    const msg = JSON.parse(data)
    
    // only reply if its a socket.request
    if (msg && msg.i) socket._sendReject(msg.i, new Error(Errors.RateLimited))

    return // stop here and prevent calling the oldHandler
  }

  oldHandler.call(socket, data)  // proceed to call the actual _onMessage from your lib
}

This only really works because this is a hook that specifically needs to run before one of your internal functions. If this hook was afterwards, I'd probably have to copy paste your entire _onMessage function. But this works out nicely for me, so I count it as a success 😃

If you're looking to extend the framework, I guess you could allow people to pass a beforeOnMessage options (function) to the constructor and then insert the hooks into your various internal class methods. Then people can write their own custom logic and call a next() function when they're done? Just my two cents 👍

[bminer]

I've contemplated supporting multiple event listeners for a given event (each running in order), which would support the middleware concept a little bit. I've also considered adding a special event name called * (asterisk) whose event listeners would run on every inbound event / request -- this could be channel specific or global.

At any rate, I'm glad you got things sorted on your end. At point in the future, I'm sure I will revisit this topic because I use this lib for quite a few projects.