mattstyles / koa-socket

Sugar for connecting socket.io to a koa instance
242 stars 50 forks source link

Is there a way to support acknowledgements mode? #5

Closed cncolder closed 8 years ago

cncolder commented 8 years ago

I can't find where to put callback argument in io.on('event', (ctx, data, callback?)

http://socket.io/docs/#sending-and-getting-data-(acknowledgements)

mattstyles commented 8 years ago

You can access the raw socket in the connection handler, you could add a ctx.socket.on and then write code exactly how you would using raw socket.io, however, you'd lose any middleware chain and its a crap pattern given this library provides an abstraction.

Something like this would work and allow an acknowledgement to be tacked on to the data object or passed as an additional parameter.

I do have a question though, line 20 shows an automatically generated acknowledgement, but, this means that the only way you could pass data back is with a thunk which is probably unacceptable. Sorry, my question is, should there be an automatic acknowledgement?

I'm inclined to say no and let the acknowledgement require manual handling, as socket.io does.

mattstyles commented 8 years ago

Nope, I've just seen that socket.io limits to a single acknowledgement (which sounds right in my opinion) so I'll strip the automatic acknowledgement and tack it onto the context.

mattstyles commented 8 years ago

I've just added this to master.

And I realised that (of course), you can't send a function in the data object, so it works exactly the same way that socket.io does.

This needs tests and docs before publishing to npm though.

cncolder commented 8 years ago

Good job.

I try this in my code is take handler(packet, data) result as ack to client. But handler result maybe plain object, Promise or async function. So codes is long and ugly. Your code example is better.

cncolder commented 8 years ago

I found this is not easy. Bcs this.middleware.then is not a good chance to call handler. eg. There is router in middleware.

mattstyles commented 8 years ago

I'm not sure I'm following, have you got a link to the code?

cncolder commented 8 years ago

I still digging my code. I try to put koa-passport into middleware list. And put passport.authenticate('local', callback) into koa-router. This promise will resolve before callback.

But if I put passport.authenticate into middleware list directly e.g. io.use(passport.authenticate ...). It's fine. This issue is a little complex. I can't find out the reason now.

mattstyles commented 8 years ago

I've never used passport, but the example application looks pretty comprehensive.

If you want to attach socket listeners only for authenticated clients then you'll need to introduce an auth layer to the socket middleware as socket middleware and koa middleware are not shared. koa-socket used the same middleware composer as koa though, so passport should (in theory) be compatible, although you’d probably want to send some sort of unique key back once a user authenticates and use that in a socket middleware.