Add event ack functionality

[Ruby184]

Why this feature is required (specific use-cases will be appreciated)?

I would like to use websocket connection to emit event and get the result of it - it is more like request and response flow. This functionality is implemented in socket.io, sails.io, etc. It is a nice feature when you want to get rid of AJAX requests for resource CRUD in SPA and pass all data through websocket. Socket.io implementation example:

// The third "ack" argument is optional and will be called with the client's answer.
socket.emit('hello', 'world', (answer) => {
  console.log(answer) // Hi!
})

// The client code
client.on('hello', (name, ack) => {
  ack('Hi!')
})

Have you tried any other work arounds?

This kind of feature is not easily implemented as addon package, because the core functionality is required on Socket and Connection classes as well as new packet type.

Are you willing to work on it with little guidance?

Sure. Just need your approval.

Proposed API

Update current server API as follows. ~~~emit method already has the ack parameter which recieves the error when packet is not sent, so add another callback with name responseAck~~~

socket.emit('hello', 'world', (err) => {
  if (err) {
    return console.log('Packet sending error: ', err)
  }
  console.log('Packet sent!')
}, (data) => {
  console.log(data) // Hi!
})

```javascript
socket.emit('hello', 'world', (data) => {
  console.log(data)
})
```
For convenience add the promisified version to server and client of this method  with `send`, `request`, `emitWithAck`, `emitWithReply` or other name. **Another option would be just let `emit` methods untouched and implement just new Promise only  `send` methods**
```javascript
const data = await socket.send('hello', 'world')
```

And for returning the response we can only return the response data from event callback:
```javascript
socket.on('hello', (name) => {
  return 'Hi!'
})
```

## Proposed changes
- adonis-websocket-packet
  - add new packet type `ACK` with code 10
  - add optional data property `id` to packet with type `EVENT`, which presence means that `ACK` response packet is expected with the same `topic` and `id`
  - add method `ackPacket` with arguments `(topic, id, data)`
  - add method `isValidAckPacket`, which will check presence of `topic` and `id` props
- adonis-websocket
  - add new properties `this._acks = new Map()` and `this._nextAckId = 0` to `Socket` class
  - add method `Socket.send(event, data)` which wil return Promise.  Get and increment `this._nextAckId` and save resolve callback to `this._acks` map under given ID. Next generate event packet and attach this ID.
  - update `socket.serverMessage` to return a promise with all resolved responses from event callbacks
  - update `Connection._processEvent` to check if packet has `id` (ack is required) and send an `ACK` packet from first resolved response
  - add handling for `ACK` packet in `Connection._handleMessage` which will call `serverAck` method on subscription
  - add `Socket.serverAck` method, which will check if given ack ID exists in map. If so it will call the callback passing recieved data
- adonis-websocket-client
  - nearly all the changes are the same as for server above

[thetutlage]

Thanks a lot for this detailed description. I love the idea and yes we can add it to the core.

I don't like the promise version, since it will stop the entire scope of code, if no reply was ever returned from the client. For example:

await socket.send('foo', 'bar')

// all code here is waiting for acknowledgement 

I believe, it's fine to accept the 4th argument.

Also I have more concern, how it will work across the cluster? Currently AdonisJs has out of the box cluster support, where messages are broadcasted to all the process and then they send those messages to their respective client.

Never the less, we can start with the basic implementation first and then improve it over time.

[Ruby184]

No problem. Thank you for your time spent reading all of this.

Yes, you are right, the problem is when promise is waiting indefinitely for the response and it never comes, so there is no way to know, when the emit request fails silently. The solution would be to just reject promise after timeout, when connection is closed or topic is left. But this can be implemented later or as addon package so leave it optional for now.

So i will just add optional ack parameters to emit methods on both client and server without introducing send method.

According to cluster. Acknowledgement of events cannot be used when broadcasting. So broadcasting will work without any changes. Event acking is only used when emiting on a single socket connection and passing optional ack callback. Only then the event packet contains the id property, which signalizes that recieved event should be acknowledged by client / server. I will create a testing repo, check it and share it with you.

I will open a PR as soon as I update the implementation and test it properly.

[thetutlage]

Cool, I will wait for it. Another question.

If I have got it right, the acknowledgment only works when emitting to a single client and not a group of clients. Also is this the same behaviour with socket.io?

[Ruby184]

First sry for not responding for such a long period of time.

I looked into socket.io implementation. When you use broadcast and try to pass an ack callback an error is thrown (https://github.com/socketio/socket.io/blob/master/lib/socket.js#L155).

Also one more thing is what we should do when promise gets rejected from one of event callbacks?

socket.on('hello', (name) => {
  return 'Hi!'
})

socket.on('hello', (name) => {
  throw new Error('rejected')
})

Should we add one more packet type for error ACK_ERROR with type and message and make ack callbacks node.js style (error first) ? Example:

socket.emit('hello', 'world', (err, data) => {
  if (err) {
    return console.log('rejected response')
  }
  console.log(data)
})

[thetutlage]

Yes, this sounds great and throwing hard exceptions for broadcastToAll is a nice way to go about it.

[thetutlage]

@Ruby184 Did you get a chance to work on it?

[Ruby184]

Sorry, I was busy at work and forget about it. But I can definitely look into it today and open PR.

[footniko]

Also waiting for event acking :)

[gaetansenn]

Hello @Ruby184 did you advance on this feature since september ? I can help you on it if you want it !