-
Notifications
You must be signed in to change notification settings - Fork 34
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
Cls support for websockets #8
Comments
Okay, so I've found the issue. The problem is that Websocket Gateways don't respect globally bound enhancers (neither with To make CLS also work with Websockets, all you have to do is explicitly bind the @WebSocketGateway()
@UseInterceptors(/* interceptor */ ClsInterceptor, WsTenantInterceptor)
export class WebsocketGateway {
// ...
} or @WebSocketGateway()
@UseGuards(/* guard */ ClsGuard)
@UseInterceptors(WsTenantInterceptor)
export class WebsocketGateway {
// ...
} (btw, you will still need to configure the I will update the README to reflect that, but Websockets are currently supported :) |
@Papooch The docs further don't address that even if you set everything up the way you've described,
It does appear this might be something they support in the future nestjs/nest#882 A simple solution I've found for now is to inject the async handleConnection(socket: Socket, incoming: IncomingMessage) {
// handleConnection is not covered by NestJS Middleware, Guards, or Interceptors
this.clsService.run(async () => {
// Do stuff here
const authToken = getAuthTokenFromIncomingMessage(incoming);
this.clsService.set('request.authToken', authToken);
}
} The rest of the incoming messages are covered by the interceptor as you mentioned. It may be obvious, but of course the CLS values won't persist between web socket messages since there's not any asynchronous context left to contain them. So, I've been storing the values I care about in a map I added onto the socket. Then, in the interceptor, I grab them off the socket and copy them into the real cls. My app has a mixture of HTTP and WS, and this lets my app only have to check one place, the cls, for these values which I think keeps it cleaner. |
@peisenmann Thanks for the writeup, this might definitely help someone implementing the same! As for the need to wrap the call to handleConnection in CLS, that's something that will be addressed in #19 in the future. |
maybe it will help someone, I did it this way, in appmo AppModule I imported it globally
clsSetupHelper
works everywhere except
|
Facing trouble implementing this. I have a NestJS project that uses websockets. I want to use Based on @Papooch's comment I was hoping this is fixed:
I tried using this on the app module:
This on the websocket gateway:
And:
But I'm still getting the error:
|
@arko7n Are you getting that error inside of the It's hard to understand what the problem is based on just these code snippets. Please create a new issue and add as many details as possible, so I can try to assist you. |
Problem statement
http
andws
, within one application.cls.set('tenant', tenant)
. This information will then be used by the custom logger to add tenant information for each log statement created in websocket context.Solution
I'm not familiar with this repository, neither with cls nor the different NestJs communication protocols. Therefore, at the moment, I can't provide an ideas. From the perspective of a consumer of nestjs-cls, it would be great to extend the existing functionality in such a way, that cls for websockt context can be added by setting a property or by an additional import in the module that provides the websocket gateway.
Minimum reproduction
The text was updated successfully, but these errors were encountered: