Skip to content
This repository has been archived by the owner on Sep 27, 2021. It is now read-only.

Event acking #68

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Event acking #68

wants to merge 2 commits into from

Conversation

Ruby184
Copy link

@Ruby184 Ruby184 commented Sep 28, 2018

Proposed changes

Add event acking functionality for events (request - response flow). For details see feature request discussion #61. Changes are dependent on adonis-websocket-packet changes (new packet types and methods) - We first need to merge packet changes. PRs in other repositories as well as tests and documentation update will follow.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have read the CONTRIBUTING doc
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

Further comments

Server:

socket.on('hello', (name) => {
  // resolved with no value
})
// this is the ack response when all callbacks return resolved promise
// because we take first non-undefined resolved value as response
socket.on('hello', (name) => {
  return 'resolved'
})
socket.on('hello', (name) => {
  throw new Error('rejected')
})
// in Ws controller
async onHello (name) {
  return 'resolved'
}

Client:

socket.emit('hello', 'world', (err, data) => {
  if (err) {
    return console.log(err.message) // rejected
  }
  console.log(data) // 'resolved' if error was not thrown from one of event callbacks
})

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.6%) to 85.068% when pulling f106349 on Ruby184:event-ack into 1d99b9a on adonisjs:develop.

@AddoSolutions
Copy link

This looks great! Any way to get this bad boy polished off?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants