elysiajs / elysia

Ergonomic Framework for Humans
https://elysiajs.com
MIT License
10.27k stars 219 forks source link

Fix generators, use `text/event-stream` protocol #743

Open remorses opened 3 months ago

remorses commented 3 months ago

Fix #742 Fix https://github.com/elysiajs/elysia/issues/741

remorses commented 3 months ago

What do you think @SaltyAom is this a breaking change or a bug fix? Previously the response was not following the text/event-stream protocol

luismanfroni commented 2 months ago

Don't know if it's related, but having a better way to catch Aborts would be really great, instead of having to do something like request.signal.addEventListener("abort", cb) (which is kinda ugly) inside all request generators handlers for cleanups. Would be cool an "abort" hook, or something like that.

remorses commented 2 months ago

@SaltyAom do you need any help with the review?

I am already using this fork in production and it works great. Another problem of the current approach is that most servers won't flush the body until they find a new line, using the right text/event-stream protocol fixes that

kravetsone commented 2 months ago

Привет, спасибо большое за твой PR. Было бы здорово, если бы ты помог мне с этими изменениями и elysiajs/eden#114 , после этого давайте сделаем это слияние.

Yay! Thanks for the work)

HK-SHAO commented 2 months ago

@SaltyAom @remorses In fact I don't think it should be up to elysia stream to handle exactly how to send the streams of the SSE interface, that should be left to the developer. It should be left to the developer, because exactly how to send each id, event, etc. should be set by the developer.

For example I would like to handle the sent id by itself, it contains some of my rules, for example the id contains a timestamp, server id and a pointer. Or I want messages sent for different events to have different event names. Another example is that I may not need the id and event because ChatGPT's stream response only returns data and does not contain the id and event. there is also a case where I only want to send a single comment. In my opinion, all the above scenarios mean that the generator should only be responsible for sending the data as is, and not have to convert it into an SSE response.

remorses commented 2 months ago

@HK-SHAO if you want customization you will need to implement it from scratch


app.get('/events', async ({ set }) => {
  set.headers['Content-Type'] = 'text/event-stream'
  set.headers['Cache-Control'] = 'no-cache'

  const stream = new ReadableStream({
    async start(controller) {
      for (let count = 0; count < 10; count++) {
        const event = `data: Event ${count}\n\n`
        controller.enqueue(event)
        await Bun.sleep(1000)
      }
      controller.close()
    }
  })

  return new Response(stream)
})
kravetsone commented 2 months ago

@HK-SHAO if you want customization you will need to implement it from scratch

app.get('/events', async ({ set }) => {
  set.headers['Content-Type'] = 'text/event-stream'
  set.headers['Cache-Control'] = 'no-cache'

  const stream = new ReadableStream({
    async start(controller) {
      for (let count = 0; count < 10; count++) {
        const event = `data: Event ${count}\n\n`
        controller.enqueue(event)
        await Bun.sleep(1000)
      }
      controller.close()
    }
  })

  return new Response(stream)
})

Yeah i agree. Elysia should folows specs but if you need sometning custom you can do it