Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onSend hook not being called on web socket route #203

Open
michele-tonizzo opened this issue May 15, 2022 · 13 comments
Open

onSend hook not being called on web socket route #203

michele-tonizzo opened this issue May 15, 2022 · 13 comments
Labels
bug Confirmed bug documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@michele-tonizzo
Copy link

michele-tonizzo commented May 15, 2022

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

@mcollina
Copy link
Member

The documentation is incorrect and onSend will not be called.

@mcollina mcollina transferred this issue from fastify/help May 16, 2022
@mcollina mcollina added bug Confirmed bug good first issue Good for newcomers documentation Improvements or additions to documentation labels May 16, 2022
@Ekott2006
Copy link

Hello, can i try to fix this issue??

@airhorns
Copy link
Member

airhorns commented Jun 7, 2023

@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.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jun 7, 2023

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.

return handler.call(this, request, reply)

@Ekott2006
Copy link

So, what's next??

@airhorns
Copy link
Member

airhorns commented Jun 8, 2023

@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.

@michele-tonizzo
Copy link
Author

@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.

@airhorns
Copy link
Member

airhorns commented Jun 8, 2023

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

@michele-tonizzo
Copy link
Author

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

@airhorns
Copy link
Member

airhorns commented Jun 8, 2023

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

@michele-tonizzo
Copy link
Author

Yes exactly

@airhorns
Copy link
Member

airhorns commented Jun 8, 2023

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.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jun 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug documentation Improvements or additions to documentation good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants