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

Server-Sent Events support #4826

Closed
soyuka opened this issue May 20, 2020 · 21 comments
Closed

Server-Sent Events support #4826

soyuka opened this issue May 20, 2020 · 21 comments

Comments

@soyuka
Copy link
Contributor

soyuka commented May 20, 2020

Feature Request

Describe the solution you'd like

Considering that Nest supports Websocket out of the box, would you like to see Server-Sent Events available as well?
There are lots of cases where using a simple EventSource client-side is sufficient over adding a whole new protocol to the stack (Websocket).

Teachability, Documentation, Adoption, Migration Strategy

On a first thought, and as it's available on top of HTTP, one could have an api like this:

// follows https://www.w3.org/TR/eventsource/#eventsource
interface MessageEvent {
    data: string;
    id: string;
    type: string;
    retry?: number;
}

@Controller('updates')
class Updates {
  @Sse()
  publishUpdates(): Observable<Partial<MessageEvent>> {
    return of({data: "Hello"}, {data: "world!"})
  }
}

On the client side:

const es = new EventSource('/updates')
// outputs
// - Hello
// - world!
es.onmessage = (message.data) => { console.log(message.data) }

Sse directive would take an optional prefix and handle the sse stream (could use https://github.com/EventSource/node-ssestream although the maintenance isn't my favorite I tend to use a custom transformer), it'd handle the id if needed.

I'd be glad to contribute such a feature if you guys are up for it!

@soyuka soyuka added needs triage This issue has not been looked into type: enhancement 🐺 labels May 20, 2020
@kamilmysliwiec kamilmysliwiec added scope: core and removed needs triage This issue has not been looked into labels May 23, 2020
@kamilmysliwiec
Copy link
Member

Looks like a nice abstraction!
Would you like to create a PR for this? However, I'd prefer not to use any additional library for this. If there's anything we must implement, let's do it as a part of the PR instead of bringing external packages.

soyuka added a commit to soyuka/nest that referenced this issue May 25, 2020
soyuka added a commit to soyuka/nest that referenced this issue May 25, 2020
@underfisk
Copy link

@soyuka do you plan to use the standard nodejs http? Because in case we have a fastify app or express we could have this SEE but I'm not sure if you need to receive an Adapter like Nestjs does

@soyuka
Copy link
Contributor Author

soyuka commented May 26, 2020

Please see #4842, it's indeed based on nodejs http and it will be compatible with express or fastify adapters without changes. Indeed, we're working directly on the response stream which fastify and express are based on.

soyuka added a commit to soyuka/nest that referenced this issue May 30, 2020
@Sharique-Hasan
Copy link

Hi, is there any update on this?

@soyuka
Copy link
Contributor Author

soyuka commented Jun 11, 2020

PR is ready on my end, waiting on reviews from the nest team. Feel free to try this out: #4826 and give some feedbacks.

soyuka added a commit to soyuka/nest that referenced this issue Jun 11, 2020
@Sharique-Hasan
Copy link

@kamilmysliwiec any idea how soon are we releasing this? Our product currently needs this as soon as possible

soyuka added a commit to soyuka/nest that referenced this issue Jul 30, 2020
@matfire
Copy link

matfire commented Aug 15, 2020

same here; anyone have any updates ?

soyuka added a commit to soyuka/nest that referenced this issue Aug 18, 2020
soyuka added a commit to soyuka/nest that referenced this issue Aug 19, 2020
soyuka added a commit to soyuka/nest that referenced this issue Aug 25, 2020
soyuka added a commit to soyuka/nest that referenced this issue Oct 13, 2020
@kamilmysliwiec
Copy link
Member

Added in 7.5.0

@Sharique-Hasan
Copy link

@kamilmysliwiec @soyuka where can we see the documentation for this?

@underfisk
Copy link

@Sharique-Hasan Check the documentation repository but it might not be added yet

@soyuka
Copy link
Contributor Author

soyuka commented Nov 2, 2020

Hi documentation is in nestjs/docs.nestjs.com#1387

A small example script is available at #4842

Last but not least I created a sample application in https://github.com/nestjs/nest/tree/master/sample/28-sse

@vh13294
Copy link

vh13294 commented Nov 3, 2020

If we use pm2 to spawn multiple instance of app, will there be multiple open connection as well ?

Another question is can pass in params to identify unique user in url ?

@soyuka
Copy link
Contributor Author

soyuka commented Nov 8, 2020

Another question is can pass in params to identify unique user in url ?

Probably but I'd suggest to use cookies for authentification or use the mercure protocol if things get complicated, there's an implementation in node https://github.com/Ilshidur/node-mercure also.

If we use pm2 to spawn multiple instance of app, will there be multiple open connection as well ?

The instance called will open a connection.

@kuriel-trivu
Copy link

kuriel-trivu commented Nov 29, 2020

How UseGuards can be combinated with Sse ?

import { AuthGuard } from '@nestjs/passport';
...
@UseGuards(AuthGuard())
@Sse('events')
methodHere() {
    ...
}

i got unexpected 401 error on client side:

const subscription = new EventSource(`${env.API}/events`);
subscription.onerror = (error) => {
    console.log({error});
};

@kuriel-trivu
Copy link

i fixed it on client request by adding headers like this:

const subscription = new EventSource(`${env.API}/events`, {
                headers: {
                    'Authorization': 'Bearer ' + token,
                }
 });

so

import { AuthGuard } from '@nestjs/passport';
...
@UseGuards(AuthGuard())

works like a charm!!

@quezak
Copy link

quezak commented Apr 21, 2021

@kuriel-trivu can you post some sources about passing headers in the EventSource constructor?
From what I see, there's only a withCredentials option for CORS, no info on headers.

@soyuka
Copy link
Contributor Author

soyuka commented Apr 21, 2021

withCredentials makes use of cookies IIRC. Many polyfills (as https://www.npmjs.com/package/event-source-polyfill) add support t for headers, as SSE is plain HTTP it'll work :).

@thematan
Copy link

thematan commented Jul 8, 2021

It seems that using sse with fastify disables CORS for that endpoint.
Tried specifying @Header decorator - but it didn't work and is bad anyway (must specify only one domain).

I injected response object and set header manually.
I guess there is a bug in the @sse decorator that erases other headers

@brianorwhatever
Copy link

@thematan I am also running into a problem with SSE and fastify CORS however am not having luck inject response obj and setting manually. Can you show me how to do that?

@thematan
Copy link

thematan commented Sep 10, 2021

@brianorwhatever

import { ServerResponse } from 'http';

@Sse("verify-payment")
	verifyPayment(@Res() response: { raw: ServerResponse }, @Query("tx") tx?: string): Observable<{ data: { status: DTOBookingStage | "close", msg?: string } }> {
		response.raw.setHeader('Access-Control-Allow-Origin', BBConfig.envVars.NODE_ENV === "production" ? "https://www.example.com" : "http://localhost:3000"); //FIXME: It won't be good when i'll have staging
		return this.bookService.verifyClientPayment({}, tx);
	}

@brianorwhatever
Copy link

sweet, thanks @thematan I'll give that a go

@nestjs nestjs locked and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants