Skip to content
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

Not throwing the exception/error when using ws adapter websocket #9056

Open
5 of 15 tasks
SairamPotta opened this issue Jan 29, 2022 · 7 comments
Open
5 of 15 tasks

Not throwing the exception/error when using ws adapter websocket #9056

SairamPotta opened this issue Jan 29, 2022 · 7 comments
Labels
needs triage This issue has not been looked into

Comments

@SairamPotta
Copy link

SairamPotta commented Jan 29, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

When I am trying to throw the exception/error on conditional basis on websocket it is not sending any handshake to the client.

here is the simple code

import WebSocket from 'ws';
import {
  SubscribeMessage,
  WebSocketGateway,
  WsException,
} from '@nestjs/websockets';

@WebSocketGateway({ path: '/api' })
export class MainGateway {
  @SubscribeMessage('message')
  handleMessage(client: WebSocket, payload: any) {
    if (payload.id === 4) {
      throw new WsException('not Found');
    }

    client.send(JSON.stringify({ id: payload.id }));
  }
}

I have just investigated why not throwing error found issue in this file
https://github.com/nestjs/nest/blob/master/packages/websockets/exceptions/base-ws-exception-filter.ts#L33
https://github.com/nestjs/nest/blob/master/packages/websockets/exceptions/base-ws-exception-filter.ts#L41

For ws - websocket adapter emit method is not working.

Minimum reproduction code

https://codesandbox.io/s/admiring-hugle-o8wqf?file=/src/Event/event.gateway.ts

Steps to reproduce

No response

Expected behavior

Should throw the handshake error or close the web socket.

Package

  • I don't know. Or some 3rd-party package
  • @nestjs/common
  • @nestjs/core
  • @nestjs/microservices
  • @nestjs/platform-express
  • @nestjs/platform-fastify
  • @nestjs/platform-socket.io
  • @nestjs/platform-ws
  • @nestjs/testing
  • @nestjs/websockets
  • Other (see below)

Other package

No response

NestJS version

No response

Packages versions

"@nestjs/common": "^8.0.0",
    "@nestjs/core": "^8.0.0",
    "@nestjs/platform-express": "^8.0.0",
    "@nestjs/platform-ws": "^8.2.6",
    "@nestjs/websockets": "^8.2.6",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^7.2.0",
    "uuid": "^8.3.2",
    "ws": "^8.4.2"

Node.js version

No response

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@SairamPotta SairamPotta added the needs triage This issue has not been looked into label Jan 29, 2022
@SairamPotta
Copy link
Author

Any updates on this issue. Please provide the solution or alternative way to handle this.

@wilywork
Copy link

wilywork commented Jan 18, 2023

Hi, it's working but needs more testing. If you find a bug, please report it.

import { ArgumentsHost, Catch } from '@nestjs/common';
import { BaseWsExceptionFilter } from '@nestjs/websockets';
import { PacketType } from 'socket.io-parser';

@Catch()
export class AllExceptionsSocketFilter extends BaseWsExceptionFilter {
  catch(exception: any, host: ArgumentsHost) {
    const client = host.switchToWs().getClient();
    client.packet({
      type: PacketType.ACK,
      data: [{ error: exception?.message }],
      id: client.nsp._ids++,
    });
  }
}

EDIT: New method, working and tested:

import { ArgumentsHost, Catch } from '@nestjs/common';
import { BaseWsExceptionFilter } from '@nestjs/websockets';

@Catch()
export class AllExceptionsSocketFilter extends BaseWsExceptionFilter {
  catch(exception: any, host: ArgumentsHost) {
    const args = host.getArgs();
    // event ack callback
    if ('function' === typeof args[args.length - 1]) {
      const ACKCallback = args.pop();
      ACKCallback({ error: exception.message, exception });
    }
  }
}

@abdelhalim97
Copy link

this answer worked for me better

https://stackoverflow.com/a/62507382/11109259

@wilywork methods to find the acknowledgement methods fail sometimes

@adshrc
Copy link

adshrc commented Feb 17, 2024

My ack callback (3rd Argument) is undefined, has anybody experienced this too?

@wilywork
Copy link

wilywork commented Feb 23, 2024

realy, ACK is not always the last parameter, so I made this correction, my current code:

import { ArgumentsHost, Catch } from '@nestjs/common';
import { BaseWsExceptionFilter } from '@nestjs/websockets';

@Catch()
export class AllExceptionsSocketFilter extends BaseWsExceptionFilter {
  catch(exception: any, host: ArgumentsHost) {
    const args = host.getArgs();
    // event ack callback
    const ACKCallback = args.reverse().find(a => 'function' === typeof a);
    if (ACKCallback) {
      ACKCallback({ error: exception.message, exception });
    }
  }
}

@Papooch
Copy link
Contributor

Papooch commented Aug 6, 2024

The provided solutions so far work only with 'socket-io'. I am also looking for a solution for 'ws'.

@Papooch
Copy link
Contributor

Papooch commented Aug 7, 2024

Okay, so after a bit of trial and error, I discovered a method that works with the WsAdapter (and most likely not with IoAdapter)

@Catch(TestException)
export class TestWsExceptionFilter implements ExceptionFilter {
    catch(exception: TestException, host: ArgumentsHost) {
        const client = host.switchToWs().getClient<WebSocket>();
        const response = {
            error: exception.message,
            exception,
        };
        client.send(JSON.stringify(response));
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into
Projects
None yet
Development

No branches or pull requests

5 participants