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

Support server-sent events #279

Open
2 tasks done
EvanHahn opened this issue Jan 25, 2024 · 8 comments
Open
2 tasks done

Support server-sent events #279

EvanHahn opened this issue Jan 25, 2024 · 8 comments

Comments

@EvanHahn
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

I would like this module to support server-sent events (SSE).

(If helpful, I can submit a pull request.)

Motivation

Currently, the best way to test SSE doesn't use this package at all. It seems the state-of-the-art is to start the server and use an EventSource, possibly with the eventsource package. This works but requires the server to be active, which this module tries to avoid.

It would be nice if this package had an answer for testing SSE.

This was briefly discussed in #105.

Example

There are several possible ways this API could be implemented. My favorite idea is a re-implementation of the W3C EventSource; something like this:

const source = new inject.EventSource(dispatch, { path: '/my-events' })

source.onmessage = (event) => {
  console.log(event)

  if (event.data === 'please close') {
    source.close()
  }
}

But there are many possible options, which we can discuss (if we even want to implement this feature at all!).

@mcollina
Copy link
Member

This would be amazing. I don't think WHAWG EventSource fits well with Node.js, I think a node stream.Readable would be better.

Something like:

const res = await inject(dispatch, { path: '/my-events', sse: true })

for await (const chunk of res) { ... }

Or analog.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 26, 2024

I dont think this is the right approach.

I considered yesterday after reading this issue to implement a sse plugin for fastify. And when I thought about it, inject method would be specific to the implementation of the plugin and not specific to light-my-request.

I implemented a fastify implementation of smee.io probot/smee.io#155

A concept would be implementing a channel concept. So you keep the open SSE connections in a Map, where key is the channel and values would be an array of connections. So you register a route to open the SSE/EventSource connection. Basically an route handler, where you return a fastify.sseCreateEventSourceStream('channelId'), where the fastify Instance was decorated with a sseCreateEventSourceStream-function which returns a stream. So when you call that path, you instantiate a connection and start to consume it. You are free to define the channelId as you like: take it from a path parameter, a header or whatever.

Then you want to send events. For this I would implement a fastify.sseEmitEvent('channelId', type, data), where the fastify instance was decorated with a sseEmitEvent function, which sends to a specific channel specific events with the provided data.

This makes it possible to send data into the EventSourceStream from other route handlers. Or like in the case of smee.io on POST /:channelId sending the serialized body to the channelId.

Then you would only need to use a pub sub method, like in my smee.io implementation, to share the payload between multiple fastify instances.

If it would be implemented like I propose it, it would not need any changes in light-my-request. And emitting sseEvents could be done via the sseEmitEvent method.

Just my 2 cents.

@EvanHahn
Copy link
Author

I considered yesterday after reading this issue to implement a sse plugin for fastify. And when I thought about it, inject method would be specific to the implementation of the plugin and not specific to light-my-request.

@Uzlopak That seems like it'd work if this module were being used exclusively with Fastify, but this module can (theoretically) be used with any Node web server. I still think it's useful for this module to support SSE in some way.

I'm new to the Fastify world, so I may be totally wrong here.

@EvanHahn
Copy link
Author

EvanHahn commented Jan 26, 2024

Some thoughts about the API design for this module (not Fastify overall):

  • SSE doesn't quite work like a normal request:
    • You can't set method (and therefore can't set payload)
    • The response won't necessarily have a full payload
    • signal doesn't really make sense because you can close the EventSource at any time
    • You might wish to set some SSE-specific options, such as the Last-Event-ID or reconnection settings
  • Users should be able to close the connection at any time.
  • Users should be able to add event listeners for message and error. They should also probably be able to listen to custom events.
  • Users should be able to detect why a connection was closed, if it was closed by the server. This might happen immediately but might happen after receiving many messages.

I think this merits a new method, rather than adding an option to inject. I also think this shouldn't use a typical Response object, because it doesn't really make sense.

I'd start by creating inject.sse, which returns an SSE source object:

// inject.sse returns synchronously.
// It takes most of the same options as `inject`, but some are removed.
const source = inject.sse(dispatch, { path: '/my-events' })

You can listen to various events:

source.on('error', (err) => {
  console.warn('An error occurred', err)
})
source.on('message', ({ data, id, event, retry }) => {
  console.log(`Message ID ${id}: event`)
})
source.on('custom-message', ({ data, id, event, retry }) => {
  console.log('Got a custom message')
})
source.on('open', () => {
  console.log('Event source opened')
})
source.on('close', (reason) => {
  console.log(`Event source was closed for reason ${reason}`)
})

You can also use Node's built-in on to achieve an iterable style:

import { on } from 'node:events';

// Get all messages until a stopping point.
// You can also listen to custom messages the same way.
const messages = []
for await (const message of on(source, 'message')) {
  messages.push(message)
  if (message.data === 'please stop') break
}

When you're all done (whenever you want), you close the source. (I don't love this and would love it to be done automatically if it isn't; perhaps there's a way to do this with FinalizationRegistry.)

source.close()

What do you think?

@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 26, 2024

Ok, granted that light-my-source is framework independent. But i have trouble to understand why a mock eventsource server should be part of this module.

@EvanHahn
Copy link
Author

Apologies for any confusion. I don't think a mock SSE server should be part of this module, but I think a mock SSE client/consumer should be.

@Uzlopak
Copy link
Contributor

Uzlopak commented Jan 26, 2024

I really dont understand the use case.

The reason of light my request is to inject a request to the instance and then the handler returns a response.

If we want to be close to the real world, it means we make a request against a path (inject) and we should get a response which is basically a eventsource server.

You say, that this is not what you need. You basically want a mocked eventsource client, where you can emit any event you want.

Lets say we implement a mock client, which should be actually really easy, because I implemented a real client in undici last week and basically can be used as a blue print and with small modifications it should work as a mock.

Now. What real world case would we actually make testable by having a mock client?

It doesnt even touch any logic of the underlying web server. And the only thing i could imagine would be testing a browser eventsource client, which is not even the target of this package.

@EvanHahn
Copy link
Author

I want to test SSE endpoints without starting the server.

Currently, our code starts the web server on a local port, creates an EventSource object to gather events, and then stops the server. This means we have special workaround code for SSE.

Ideally, we'd be able to run our SSE tests just like we run our others: without starting a web server.

Again, apologies if I'm not understanding something here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants