fastify-websocket icon indicating copy to clipboard operation
fastify-websocket copied to clipboard

onSend hook not being called on web socket route

Open michele-tonizzo opened this issue 3 years ago • 14 comments

Hello,

I'm filtering the data returning from a route before being sent back to the client using onSend hook and works as expected

fastify.addHook("onSend", (request, reply, payload, next) => {
    const mask = request?.query?.["mask"];

   // ...

    const filtered = applyMask(object, mask);

    return next(null, JSON.stringify(filtered));
  });

  return next();
};

I also have web socket connections in my API that I would like behave the same before sending the message to the client but apparently, even if the in documentation of @fastify/websocket it states that "it will run any hooks that have been registered", it does not apply to onSend hook when sending back using connection

connection.socket.send(JSON.stringify(data));

From testing the hook is not called entirely.

Is this the correct behaviour? How can I achieve the same result that I have using HTTP methods?

Thanks

michele-tonizzo avatar May 15 '22 18:05 michele-tonizzo

The documentation is incorrect and onSend will not be called.

mcollina avatar May 16 '22 08:05 mcollina

Hello, can i try to fix this issue??

Ekott2006 avatar Jun 07 '23 08:06 Ekott2006

@Ekott2006 I don't think it really makes sense to call the onSend hook, as the webhook can send and receive data many times, unlike a normal request. I think the right resolution here is to update the documentation to describe which hooks do run and which are skipped once the request has been upgraded to a websocket.

airhorns avatar Jun 07 '23 13:06 airhorns

I guess, we would need to implement a new "hook" stage like calling it "onWebsocketSend". And then handle the "hooks" before calling the webhook handler here.

https://github.com/fastify/fastify-websocket/blob/ec0e7aef272b869607627754ae3c978d13d8259b/index.js#L141

Uzlopak avatar Jun 07 '23 14:06 Uzlopak

So, what's next??

Ekott2006 avatar Jun 08 '23 06:06 Ekott2006

@Ekott2006 @michele-tonizzo what did you want the onSend hook for for websockets? If you can describe your use case it would help us figure out if we need to go on the big adventure to add custom hooks to fastify proper or if there is a simpler way to accomplish what you are trying to do.

airhorns avatar Jun 08 '23 14:06 airhorns

@airhorns In my case I had to apply a mask to the reply after it was sent out of the route. For example users that can read other users shipping addresses but not their billing addresses. They had a permission mask user.userId=id1,id2.shipping

There surely are alternatives to solve my problem, for example I could do everything inside the route and get the users permissions and filter the data data = applyMask(data) before sending it back to the client reply.send(data) or I could select when reading the database.

But I wanted something that could be written inside a plugin in order to have a cleaner code (at the cost of speed, selecting when querying the db is much better move). The plugin would get the mask based on the user permissions and filter out the data automatically.

michele-tonizzo avatar Jun 08 '23 15:06 michele-tonizzo

@michele-tonizzo is the reply you were sending a websocket message or a normal JSON reply?

airhorns avatar Jun 08 '23 15:06 airhorns

It was a JSON serialised into a string and sent as websocket message

michele-tonizzo avatar Jun 08 '23 16:06 michele-tonizzo

Ah, ok. And I take it you were expecting the onSend hook to be called for each message sent out across the websocket?

airhorns avatar Jun 08 '23 16:06 airhorns

Yes exactly

michele-tonizzo avatar Jun 08 '23 16:06 michele-tonizzo

I think that @fastify/websocket is best seen as a low level library for handling websocket connections at all, and, a lot of different things can go over websocket connections. JSON serialized strings yes, but also audio data, database protocols, really any bytes. So, I don't think that @fastify/websocket should make any assumptions about the format of the payloads going across the wire, which it would need to do to to provide that kind of API. I also think that it'd still make sense to use something other than onSend for that kind of hook, as I don't think its wise to couple the processing of full HTTP responses to tiny messages in a potentially stateful protocol. I get the need for a clean way to transform messages as they go in and out of the socket, but I think that's a problem that's general to any websocket connection, and depends a lot on the transport protocol, so I'd vote for not including any hooks in @fastify/websocket to do this kind of thing. There's probably some libraries on NPM for adding stacks of transforms on top of an existing websocket.

airhorns avatar Jun 08 '23 16:06 airhorns

The onSend hook is not called because it is not part of the lifecycle of the websocket connection. Thats why I proposed to expose the hook functionality so that we can process them in the websocket connection.

But for that I would add the functionality to have custom hooks defined, so that we not confuse onSend of the websocket http request connection and the onSend of the websocket communication.

Uzlopak avatar Jun 08 '23 16:06 Uzlopak