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

koa responding with a 404, with all other middlewares turned off, even with respond = false #6

Open
cdaringe opened this issue Feb 3, 2020 · 4 comments

Comments

@cdaringe
Copy link

cdaringe commented Feb 3, 2020

problem

new WebSocket('wss://localhost:7777') from a browser

yields

VM127:1 WebSocket connection to 'ws://localhost:7777/' failed: Error during WebSocket handshake: Unexpected response code: 404

when using only this middleware

discussion

i see that respond = false, but then again: https://github.com/koajs/koa/blob/master/docs/api/context.md#ctxrespond

@cdaringe
Copy link
Author

cdaringe commented Feb 3, 2020

oh, one must use the socket during the http upgrade req/res connection for the ws connection to establish itself. hmm. so it's a socket that can only exist during the lifecycle of the request/response? is that correct? if so, why use a socket at all then?

alt, is this line supposed to be one block out, one line down?

ctx.respond = false

@b3nsn0w
Copy link
Owner

b3nsn0w commented Feb 3, 2020

I don't really know how you got this issue without seeing how you tried to use ctx.ws(). Gonna try to give you a few pointers though, hope it helps.

The ctx.ws() function is added when the request is recognized as a WebSocket upgrade request, but it only handles (and consumes) the request if it's actually called. If you don't call it, the request is left unhandled, and handling it falls back to the rest of your middleware chain, most likely resulting in a 404.

You must retrieve the socket while the upgrade request is still alive, that's just the way HTTP works. It doesn't matter if you do it by calling ctx.ws() or if you handle it yourself, but if you time the request out or send back a different response, the WebSocket protocol just won't work.

You could probably add ctx.respond = false in your own code at some point, but this is likely a workaround and doesn't fix your root issue. In the ctx.ws() function, this line is there because the ws server will respond to the request, so Koa doesn't need to (and in fact shouldn't) do it.

Could you post some example code to show how you're encountering this issue?

@cdaringe
Copy link
Author

cdaringe commented Feb 9, 2020

The ctx.ws() function is added when the request is recognized as a WebSocket upgrade request, but it only handles (and consumes) the request if it's actually called.

right. everything you said makes sense, but i suppose i'm questioning the value behind it. it just doesn't seem that useful that ctx.ws is only available during one single request/response lifecycle--specifically the upgrade http req/res--and no subsequent requests. websockets are intrinsically valuable for async comm that don't necessarily fit the req/res pattern, but as written, .ws() will only exist during a classico http request, feeling like an unnecessary protocol switch (at least for browser clients). of course i, the user, could capture that socket reference and persist it into some sort of cache of my choice, but i suppose i expected this lib to help with that, even if minimally, in some fashion. as it stands, and as the examples show, the socket is kind of a one-time use thing. if the client is the browser, who will probably only try to establish a connection once onload, one could reason that easy-ws won't help very much for interacting with that page. i'm more-or-less using this comment to think out loud :) as i work through this.

@b3nsn0w
Copy link
Owner

b3nsn0w commented Feb 9, 2020

Well, if you'd like to use the socket outside of your request handler function, you'll need to move it somewhere. I tend to just do something like this:

const handler = async (ctx) => {
  if (ctx.ws) {
    const ws = await ctx.ws()

    ws.on('message', (message) => {
      // react to incoming messages here
    })

    // send any initial messages if needed
    ws.send('hello there')

    // also, don't handle the request in any other way, ctx.ws() already does it
    return
  }

  ctx.body = 'this is a websocket route'
  ctx.response.status = 400
}

But if you prefer to have your event handler somewhere else, you can just move the retrieved ws object out of that function. You do have to retrieve it within the request/response lifecycle, but you can use it longer (the event listener in the above example stays open beyond that, for instance). This library doesn't limit your WebSocket connection's lifespan in any way.

I suppose this is the kind of cache you would like to use there. Honestly, I don't think this library will ever help you with that, because that would require making assumptions about your code structure. If this is a frequent use case for you, you could consider writing a companion module to use with this one. Maybe something like this?

const handler = async (ctx) => {
  if (ctx.ws) {
    const ws = await ctx.ws()
    yourCache.addSocket(ws)

    // no need to handle the request in any other way
    return
  }

  ctx.body = 'this is a websocket route'
  ctx.response.status = 400
}

As for why ctx.ws() is a function instead of just a value, it's just the principle to not modify the request unless your handler explicitly triggers it.

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

2 participants