fastify / fastify-websocket

basic websocket support for fastify
MIT License
393 stars 72 forks source link

connection.send behaves differently on Fastify v4 #209

Closed naknode closed 2 years ago

naknode commented 2 years ago

Prerequisites

Last working version

3.29.0

Other packages

'@fastify/websocket': 6.0.1 '@fastify/cors': 8.0.0

Stopped working in version

4.0.0

Node.js version

18.3.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Fedora 35

💥 Regression Report

I have this simple server:

const fastify = require("fastify")();
fastify.register(require("@fastify/websocket"));
fastify.register(require("@fastify/cors"));

fastify.get("/", { websocket: true }, (connection) => {
  console.log("Someone connected.");

  connection.socket.send("Hello from the server!");

  connection.socket.on("message", (message) => {
    console.log("Server was pinged: ", message.toString());

    connection.socket.send("Hello Fastify WebSockets");
  });
});

fastify.listen({ port: 9000 }, (err, address) => {
  if (err) {
    console.error(err);
    process.exit(1);
  }
  console.log(`Server listening at: ${address}`);
});

I run it using fastify@3.29.0 and it all works. I upgrade to version 4 and when I visit it on the client (http://localhost:9000):

{
  "statusCode": 500,
  "error": "Internal Server Error",
  "message": "connection.socket.send is not a function"
}

Same thing when I go to Postman and I connect via it's websocket-type request (ws://localhost:9000). It just disconnects.

It seems to have a problem with connection.socket.send("Hello from the server!");. My goal is that i try to send some data from the server upon connection.

This use to work in fastify@3.x.x

Steps to Reproduce

  1. Run it using nodemon
  2. Observe terminal
  3. Connect to ws://localhost:9000 on Postman or connect to Client
  4. See no activity.
  5. Downgrade fastify to version 3.29.0. Observe success.

Expected Behavior

I expect the server to successfully connect and the fastify route / to start accepting socket messages.

climba03003 commented 2 years ago

Please explicitly specify the host to either 127.0.0.1 or ::1 to see if it is duplicate of https://github.com/fastify/fastify-websocket/issues/207

naknode commented 2 years ago

Please explicitly specify the host to either 127.0.0.1 or ::1 to see if it is duplicate of fastify/fastify-websocket#207

@climba03003 OK, I update it to this line: fastify.listen({ port: 9000, host: "127.0.0.1" }, (err, address) => { and now I get: Server listening at: http://127.0.0.1:9000 while the server sees my connection "Someone connected.", it doesn't work 100% because it immediately closes right after connection. Postman is the same way.

image

With I have the config like this: { port: 9000 }, with no host specified, the console says: Server listening at: http://[::1]:9000 and I get that 500 Internal Server Error.

I still get the connection.socket.send is not a function error going to http://127.0.0.1:9000

climba03003 commented 2 years ago

I see the problem now. You must await the fastify.register(require("@fastify/websocket")).

climba03003 commented 2 years ago

I think it worth to update the document. Would you like to send a PR to address the issue?

naknode commented 2 years ago

Ugh. I need some sleep. Thanks.

Yeah I can do that. What updates are you looking for? Just making sure that the examples work with node 18.x.x?

climba03003 commented 2 years ago

Ugh. I need some sleep. Thanks.

No rush. Take your time.

I explain more in here. In fastify@4, the addHook('onRoute') inside plugin is registered in async, so any plugin registered a onRoute hooks is required to wait before the route registration can take effect on it.

That means when you want to register a route in same layer. You need to wait for the plugin initialization.

import Fastify from 'fastify'
import FP from 'fastify-plugin'

const fastify = Fastify()
// await is important here
await fastify.register(function(instance) {
  instance.addHook('onRoute', () => {})
})

fastify.get('/', () => {})

Or you can encapsulate the route inside a plugin. So, the plugin can run in order.

import Fastify from 'fastify'
import FP from 'fastify-plugin'

const fastify = Fastify()
fastify.register(function(instance) {
  instance.addHook('onRoute', () => {})
})
fastify.register(function(instance) {
  // this plugin receive the `onRoute` hook
  instance.get('/', () => {})
})
lostintheway commented 2 years ago

I dont get it, how do I solve this error?

mcollina commented 2 years ago

Would you like to send a PR to address the issue

@climba03003 I checked the README and it's using the correct patterns everywhere.

mcollina commented 2 years ago

I dont get it, how do I solve this error?

@lostintheway use await fastify.register(require('@fastify/websocket'))

climba03003 commented 2 years ago

Would you like to send a PR to address the issue

@climba03003 I checked the README and it's using the correct patterns everywhere.

Yes, it is. But I think we need to emphasis more about the correct usage should be either await the plugin or wrap the route within plugin.

naknode commented 2 years ago

I was able to get this with awaiting.