fix(cli/dts): Make respondWith() return a Promise

[nayeemrmn]

Fixes #10101.

[bartlomieju]

I am not a fan of this change - either we preserve the API from service workers and respondWith doesn't return promise or let's just rename this function to respond. CC @ry

[bartlomieju]

I am not a fan of this change - either we preserve the API from service workers and respondWith doesn't return promise or let's just rename this function to respond. CC @ry

@nayeemrmn I agree with your rationale here: #10101 (comment) - I mentioned this problem to Ryan and Luca several times. I do agree that function to respond should be awaitable, but I don't think we should change signature of respondWith to make it non-compatible with Web API, hence my suggestion to name it respond

[kitsonk]

It is critical that we detect errors that occur with .respondWith() in the underlying ops... For example, I recently came across a situation where curl was "misbehaving" and causing:

error: Uncaught (in promise) Http: connection closed before message completed
    at unwrapOpResult (deno:core/core.js:100:13)
    at async Object.respondWith (deno:runtime/js/40_http.js:152:11)

While it isn't documented, .respondWith() does currently return a Promise than I can use to catch this error, without that, the error goes unhandled and the process exits, which is 😢 for a web server to crash because a client doesn't quite behave correctly.

Also, there is a need in some cases to know when .respondWith() completes, so that resources associated with any response can be cleaned up. It is difficult/challenging at times to ensure that all resources can be released (like file handles closed) until we know the server isn't going to reference them anymore.

[ry]

LGTM - tho I have to say, it seems that Service Worker standard really intends for this to be a sync function and that error handling should somehow be done in the promise passed to respondWith... That said, let's do this for now.

[jimmywarting]

While it isn't documented, .respondWith() does currently return a Promise than I can use to catch this error, without that, the error goes unhandled and the process exits, which is 😢 for a web server to crash because a client doesn't quite behave correctly.

exactly what happen to me today... totally unexpected.
thing respondWith should have stayed the way it was and then you had to catch the error in any other way instead.

just an idea:

  const httpConn = Deno.serveHttp(conn)

  for await (const requestEvent of httpConn) {
    const { request } = requestEvent
    httpConn
      .when(request)
      .upload(progressEvent => { 'calculate how much have been uploaded and how much is left' })
      .download(progressEvent => { '...' })
      .aborted(evt => { 'user aborted or closed the request' }) 
      .respondWith('2xx', evt => { })
      .respondWith('3xx', evt => { })
      .respondWith('4xx', evt => { })
      .respondWith('5xx', evt => { })
      .completed(evt => { })

There is also a request.signal that could have been used for good use also.

  const httpConn = Deno.serveHttp(conn)

  for await (const requestEvent of httpConn) {
    const { request } = requestEvent
    request.signal.onabort = evt => {
      console.log(request.signal.reason)
    }