-
Notifications
You must be signed in to change notification settings - Fork 108
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
add 'unauthenticated' event, fix 'unsubscribed' event capabilities, update missing typedefs #147
Conversation
… allow advanced logging catch ServerClient (unavailable via reporter interface)
…rverNodeId (server-initiated -ch) or clientNodeId (client-initiated -ch)
…erverClient by evicting nodeIds / clientIds later, after unsubscribe
Can you add a description for PR? |
Added some motivation |
I needed a reason of changes. Now it is great. It is minor change ( |
Not at all — already applied a patch |
server-client/index.test.ts
Outdated
@@ -257,9 +257,15 @@ it('removes itself on destroy', async () => { | |||
'10:client2': { filters: { '{}': true } } | |||
} | |||
} | |||
let unsubscribedClientNodeIds: string[] = [] | |||
test.app.on('unsubscribed', (action, meta, clientNodeId) => { | |||
unsubscribedClientNodeIds.push(clientNodeId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 space indent should be used
@@ -657,7 +657,7 @@ export class BaseServer { | |||
} | |||
} | |||
} | |||
this.emitter.emit('unsubscribed', action, meta) | |||
this.emitter.emit('unsubscribed', action, meta, clientNodeId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need extra parameter? We can use parseId(meta.id).clientId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because in case of server-side channels cleanup (e.g. client is disconnected because of timeout), nodeId is server.nodeId
Line 172 in ebc9815
let actionId = this.app.log.generateId() |
…pdate missing typedefs (#147) * add missing 'addClean' reporter type definition * add BaseServer 'unauthenticated' event on par with 'authenticated' to allow advanced logging catch ServerClient (unavailable via reporter interface) * add 'unsubscribed' event clientNodeId last arg (that can be either serverNodeId (server-initiated -ch) or clientNodeId (client-initiated -ch) * ClientServerdestroy: allow 'unsubscribed' event handler to retrieve ServerClient by evicting nodeIds / clientIds later, after unsubscribe * fix indent
…pdate missing typedefs (#147) * add missing 'addClean' reporter type definition * add BaseServer 'unauthenticated' event on par with 'authenticated' to allow advanced logging catch ServerClient (unavailable via reporter interface) * add 'unsubscribed' event clientNodeId last arg (that can be either serverNodeId (server-initiated -ch) or clientNodeId (client-initiated -ch) * ClientServerdestroy: allow 'unsubscribed' event handler to retrieve ServerClient by evicting nodeIds / clientIds later, after unsubscribe * fix indent
…pdate missing typedefs (#147) * add missing 'addClean' reporter type definition * add BaseServer 'unauthenticated' event on par with 'authenticated' to allow advanced logging catch ServerClient (unavailable via reporter interface) * add 'unsubscribed' event clientNodeId last arg (that can be either serverNodeId (server-initiated -ch) or clientNodeId (client-initiated -ch) * ClientServerdestroy: allow 'unsubscribed' event handler to retrieve ServerClient by evicting nodeIds / clientIds later, after unsubscribe * fix indent
Found that addClean reporter typedef is missing. Fixed.
Also while implementing suitable logging in my app I found out there are some bits missing: