feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
15k stars 743 forks source link

uWebSockets.js #3063

Open joacub opened 1 year ago

joacub commented 1 year ago

I was wondering if there is any plan to support uWebSockets.js ??

it can be a huge improvement. Btw thanks for your hard work on the project I really like it

8BallBomBom commented 1 year ago

Should already be supported in Feathers v5 due to the updated modules. Can't say i've tested it properly and it doesn't seem to be documented currently. But here goes.

Make sure you have uWebSockets.js installed. npm install uWebSockets.js@uNetworking/uWebSockets.js#v20.31.0 yarn add uWebSockets.js@uNetworking/uWebSockets.js#v20.31.0 pnpm install uWebSockets.js@uNetworking/uWebSockets.js#v20.31.0

Then add these changes to your projects app.ts file.

import { App } from 'uWebSockets.js';

app.configure(
    socketio( {
        cors: {
            origin: app.get( 'origins' )
        }
    },
    io => {
        io.attachApp( App() );
    } )
);

Far as i can see that seems to be working?

socket.io:server creating uWebSockets.js-based engine with opts {"cleanupEmptyChildNamespaces":false,"path":"/socket.io"} +0ms

Not 100% sure if that is the correct way to handle things or if that even works properly. Definitely a start 😅

joacub commented 1 year ago

Should already be supported in Feathers v5 due to the updated modules. Can't say i've tested it properly and it doesn't seem to be documented currently. But here goes.

Make sure you have uWebSockets.js installed. npm install uWebSockets.js@uNetworking/uWebSockets.js#v20.19.0 yarn add uWebSockets.js@uNetworking/uWebSockets.js#v20.19.0

Then add these changes to your projects app.ts file.

import { App } from 'uWebSockets.js';

app.configure(
  socketio( io => {
      io.attachApp( App() );
  } )
);

Far as i can see that seems to be working?

socket.io:server creating uWebSockets.js-based engine with opts {"cleanupEmptyChildNamespaces":false,"path":"/socket.io"} +0ms

Not 100% sure if that is the correct way to handle things or if that even works properly. Definitely a start 😅

Hi sir thanks for your comment, I don’t think that gonna work, the server never gets up due to the listening función gets called once in the express or Koa server and then again uWebsockets. UWebsockets is a full replacement of both server and websockets.

I’m also not 100% sure but I already tried your approach and never works. I just tried again and get errors in the subscription function in the socket.io lib

8BallBomBom commented 1 year ago

Was following how it is stated as supported with the latest SocketIO versions. Have a look over here.

From basic testing the changes i made did indeed change the server engine instance over to uWS but i haven't tried using any clients with websockets to confirm functionality.

As for getting errors on listening? Not experiencing any of that but i did change a few things. If you are using Koa then you need to use the app.callback function for https, have a look here.

My app index file looks like this.

import https from 'https';
import fs from 'fs';

import { app } from './app';
import { logger } from './logger';

const port = app.get( 'port' );
const host = app.get( 'host' );

if ( process.env.NODE_ENV === 'production' ) {
    const server = https.createServer( {
        key: fs.readFileSync( 'ssl/private.key' ),
        cert: fs.readFileSync( 'ssl/certificate.pem' )
    }, app.callback() ).listen( port );

    server.on( 'listening', () => logger.info( `API Started - https://${host}${(port !== 443) ? `:${port}` : ''}` ) );
    app.setup( server );
} else {
    const server = app.listen( port );

    server.then( () => logger.info( `API Started - http://${host}${(port !== 80) ? `:${port}` : ''}` ) );
};

process.on( 'unhandledRejection', ( reason, p ) =>
    logger.error( 'Unhandled Rejection at: Promise ', p, reason )
);

But as you said, Koa or Express overrides the functionality? Can't confirm without testing. Though it would appear that the changes above would only push the websocket handling over to uWS. Nothing more, unsure how we'd go about letting uWS cover everything else.

@daffl Can you provide help please? Not sure why this functionality hasn't been documented and unsure if this implementation is actually plausible without changes to other internal systems.

joacub commented 1 year ago

The thing is that Koa or express will get up the service in the same port as the socket.io will does in case of this or will never get up the server cause listening will never be fired by the app itself and will be fired by other server.

8BallBomBom commented 1 year ago

Without testing in depth all i can see based on printing is that the normal operation of the server is the same apart from the sockets backend handling for SocketIO is replaced with uWS.

As for expanding to have uWS also replace the web server? From what i can tell that would require some other changes.

Looking back on things, it is essentially working as it used to where you'd just specify the wsEngine. Have a look at this, it is an older version of uWS but repurposed.

daffl commented 1 year ago

Socket.io works standalone as well, you can just remove the Koa or Express integration pieces. Adapted from the quick start this should work (though I'm getting a uws build error for some reason):

import { feathers } from '@feathersjs/feathers'
import socketio from '@feathersjs/socketio'
import '@feathersjs/transport-commons'
//@ts-ignore
import { App } from 'uwebsockets.js'

// This is the interface for the message data
interface Message {
  id?: number
  text: string
}

// A messages service that allows us to create new
// and return all existing messages
class MessageService {
  messages: Message[] = []

  async find() {
    // Just return all our messages
    return this.messages
  }

  async create(data: Pick<Message, 'text'>) {
    // The new message is the data text with a unique identifier added
    // using the messages length since it changes whenever we add one
    const message: Message = {
      id: this.messages.length,
      text: data.text
    }

    // Add new message to the list
    this.messages.push(message)

    return message
  }
}

// This tells TypeScript what services we are registering
type ServiceTypes = {
  messages: MessageService
}

// Creates an ExpressJS compatible Feathers application
const app = feathers()

// Configure Socket.io real-time APIs
app.configure(
  socketio((io) => {
    io.attach(App())
  })
)
// Register our messages service
app.use('messages', new MessageService())

// Add any new real-time connection to the `everybody` channel
app.on('connection', (connection) => app.channel('everybody').join(connection))
// Publish all events to the `everybody` channel
app.publish((_data) => app.channel('everybody'))

// Start the server
app.listen(3030).then(() => console.log('Feathers server listening on localhost:3030'))

// For good measure let's create a message
// So our API doesn't look so empty
app.service('messages').create({
  text: 'Hello world from the server'
})
8BallBomBom commented 1 year ago

Thanks for the help 👍🏻 Bit unusual, not experiencing any errors on this end. Question is, would it be worth allowing uWS to act as the http/https server alongside handling sockets? Not sure on the performance benefits but could be worth looking into 😮

joacub commented 1 year ago

uws is really performant so yes It will be really better than not using them, the idea is to replace the full integration with uws and relay on them uniquely as feathers is doing with express or koa (http node).

I didn't test what @daffl just write, I will do a try this night to see if that way will works. but the idea is to replace as feathers is doing today with the express piece and the koa piece, something very similar (uws replace express and koa or any other server).

I already replace them koa and express and it works just for test purposes, that needs deeper integration with feathers and app.use like middleware system.

daffl commented 1 year ago

Feathers already has a middleware system through hooks. I've also gotten pretty weary of adding new integrations because the problem is that 95% of people will use the default anyway but maintaining it will be 200% the work for the other 5%. I'm pretty sure this should already work like I posted. If not, it's definitely worth fixing. If someone wants to put in the work of adding this as a core module, write tests, update the CLI and documentation and guides they are welcome to but other platforms like Deno, Cloudflare etc. are higher on my personal priority list.