nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
66.89k stars 7.56k forks source link

app.useWebSocketAdapter() causes 404 No 'Access-Control-Allow-Origin' #307

Closed nick-allen closed 6 years ago

nick-allen commented 6 years ago

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Sockets/Gateways works as expected with default app config. Calling the app.userWebSocketAdapter() method with any adapter, including the builtin IoAdapter, causes the following error, and the socket never connects:

Failed to load http://localhost:3000/socket.io/?EIO=3&transport=polling&t=M1Eh3gz: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost:4200' is therefore not allowed access. The response had HTTP status code 404.

Minimal reproduction of the problem with instructions

import { IoAdapter } from '@nestjs/websockets';

export async function bootstrap(): Promise<void> {
  const app = await NestFactory.create(ApplicationModule);

  app.useWebSocketAdapter(new IoAdapter());

  await app.listen(3000);
}

bootstrap();

Environment


Nest version: 4.4.2
nick-allen commented 6 years ago

GET /socket.io/ 404

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Cannot GET /socket.io/</pre>
</body>
</html>
nick-allen commented 6 years ago

Figured it out, it's happening since the IoAdapter is expecting to get the app's httpServer. With the default app setup, it's initialized using the app's private httpServer property, which can't be accessed outside the app until after app.listen() resolves, making it impossible to (without forcing it) configure a modified IoAdapter instance with the app's server instance.

Was able to force it doing the following:

import { IoAdapter } from '@nestjs/websockets';

class MyIoAdapter extends IoAdapter {
  createIOServer(port: number) {
    const io = super.createIOServer(port);
    // Do something with io before returning
    return io
  }
}

export async function bootstrap(): Promise<void> {
  const app = await NestFactory.create(ApplicationModule);

  app.useWebSocketAdapter(
    new MyIoAdapter(
      // Cast to any to get around httpServer being private
      (<any>app).httpServer
    )
  );

  await app.listen(3000);
}

bootstrap();

Changing useWebSocketAdapter to expect a class instead of an instance would let the app inject the http server, or allowing the server to be accessed publicly, would remove the need for this hack.

kamilmysliwiec commented 6 years ago

Hi @nick-allen, Why are you trying to replace the default implementation with the default implementation?

app.useWebSocketAdapter(new IoAdapter());

As far as I see any other custom adapter should work fine

nick-allen commented 6 years ago

Was using it as an example for the issue. I'm trying to extend the built-in IoAdapter to capture the socket at bootstrap time. Still an issue for me since it's an extension of the built-in class that needs the same constructor arguments.

kamilmysliwiec commented 6 years ago

Ah gotcha. Maybe just a method to pick an underlying HTTP server from the Nest application instance?

nick-allen commented 6 years ago

Yep, was my last suggestion to allow access to the underlying server built from the Nest factory

kamilmysliwiec commented 6 years ago

The getHttpServer() is available since 4.5.1. Let me know if you meet any issues

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.