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

Inside event "close" cannot broadcast event. #63

Open
osddeitf opened this issue Aug 20, 2018 · 9 comments · May be fixed by #81
Open

Inside event "close" cannot broadcast event. #63

osddeitf opened this issue Aug 20, 2018 · 9 comments · May be fixed by #81
Assignees

Comments

@osddeitf
Copy link

I have recently update my code base from AdonisJS version 3.2 to the latest. With much effort, everything working right, but not new

@adonisjs/websocket

or

@adonisjs/websocket-client

package.
With old code base, a code like this should working:

Ws.channel("user", function(socket) {
    socket.on("disconnect", function() {
        socket.exceptMe().emit("closing", socket.id)
    })
}

As the code above, whenever socket connection disconnect (close browser, ...), other socket connection would be notified by event closing. But, with a new code base:

Ws.channel('user', ({ socket }) => {
    socket.on('close', () => {
        socket.broadcast("closing", socket.id)
    })
})

With event mapping from disconnect to close, and exceptMe().emit() to broadcast(), it doesn't work anymore. On the server, event close still be fire, but socket likely to not function anymore, so client could not receive closing event? I guess that this happened due to prevent not necessary emit when socket is going to be closed, I tried to find out, but I'm not good at tweaking source code to find out the problem.

My company project use AdonisJS, and my "boss" doesnt want to create a standalone socket server, so I must use builtin supported socket in AdonisJS. Please help me as soon as possible.
Thanks and best regard!

@thetutlage
Copy link
Member

It should broadcast to everyone, except the one who disconnected (which is relevant). Can you share, how exactly you are testing to know that the event is fired to all other connected clients?

@osddeitf
Copy link
Author

This is a script I created and included in template:

const Ws = require("@adonisjs/websocket-client");

const connection = Ws("").connect();
const socket = connection.subscribe("chat");
socket.on('closing', () => console.log("CATCH CLOSING"))

I'm had tried:

Ws.channel('user', ({ socket }) => {
  socket.broadcast('closing', socket.id)
})

It's successfully fired event closing, as on the first tab on browser, when open the second tab, console show up "CATCH CLOSING". But not when I included close event on server side:

socket.on('close', () => {
    socket.broadcast("closing", socket.id)
})

And try closing one tab, the other's console didn't change anything!

@thetutlage
Copy link
Member

Yup, you are right, as soon as a socket disconnects. It is not able to send messages to other connected sockets. Lemme work on the fix.

@maxsprite
Copy link

Hello, i'm newbie in JS, but i get that bug too. I interested was happening and started look at the source.

Having found the function broadcast in Socket/index.js i see call makeEventPacket in current ws connection. Debug this before and after function call - understood that function broadcast stopping with execute makeEventPacket.

In body function makeEventPacket (located in Connection/index.js) we see next if statement:

if (!this.hasSubscription(topic)) {
throw new Error('Topic ${topic} doesn't have any active subscriptions')
}

But, this if execute for broadcast and broadcastToAll functions every time. And if run broadcast or broadcastToAll inside onClose event - we already doesn't have this topic subscription and have throw new Error(...) (broadcasting will be stopping after throw error). Because this topic unsubscribe for connection before we running broadcasting yet in onClose event function.

If we comment on that if statement written above, broadcast will be work.

PS: i hope that helps for fix this bug.

@MattGould1
Copy link

@thetutlage was this resolved? Believe I have same issue

@mllull
Copy link

mllull commented Feb 20, 2019

+1 for this issue, any news about it?

In socket.io, when a client closes the window, an event called disconnectis fired. It would be nice if it can be implemented here. (More info about this: https://socket.io/docs/).

@TrlaL
Copy link

TrlaL commented Apr 15, 2019

When will this be fixed?

Flambe added a commit to intergral/adonis-websocket that referenced this issue Jan 22, 2020
…debug message

replaced the error inside makeEventPacket to a debug message as this was blocking broadcasts from
being sent when a socket connection was closed

adonisjs#63
@jona4life
Copy link

While you can't use the broadcast event inside close

You should be able to use this method:

const Ws = use('Ws');
const topic = Ws.getChannel('channel').topic('topic');
// emitTo or broadcast will work below
topic.emitTo('message', 'I\'m closing guys :-)', [socket_ids]);
// or
topic.broadcast('message', 'I\'m closing guys :-)');

This should work as you're not broadcasting through the closed socket. According to the docs, this method can be used outside the scope of a socket. So, it should work even if the socket is closed

@jcjp
Copy link

jcjp commented Aug 14, 2020

I have a workaround on this to trigger the event upon disconnect, I used jona4life's code above and tweak it a little bit:

const Ws = use('Ws');

class ChatController {
  constructor({ socket, request }) {
    this.socket = socket
    this.request = request
    this.topic = Ws.getChannel('sample').topic('sample')
  }

  onMessage(message) {
    this.socket.broadcast('message', 'Sample message from the server')
  }

  onDisconnect() {
    this.topic.broadcast('message', 'A player has disconnected')
  }
}

module.exports = ChatController

If anybody is needing this or stumble upon this.

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