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

8.0.0 IoAdapter class for socket.io integration, suggested trail to pass "forwardEventNameAs" option to bindMessageHandlers call? #6771

Open
sebastiangug opened this issue Mar 26, 2021 · 0 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@sebastiangug
Copy link

sebastiangug commented Mar 26, 2021

I'm currently using socket.io v4.0 custom adapter with nestjs and I see that the implementation in nestjs v8.0.0 will support socket.io v4.

One issue that I've had was my exception handlers had no way to get the actual event name from the socket itself or the data, which I would like to emit the error back to: .on(event_name) as opposed to a general error handler .on("ERROR").

It seemed of value to me to emit the error within the context of the event. In order to achieve this without forcing the client-side to send the event name as a data parameter OR scoping my validation pipes & errors to each request, I edited the socket.io adapter bindMessageHandlers as follows:

handlers.forEach(({ message, callback }) => {
      const source$ = fromEvent(client, message).pipe(
        mergeMap((payload: any) => {
          
          
          
          // CHANGED CODE BELOW 🢃🢃🢃🢃🢃
          
          // also pasing the message name to the this.mapPayload() call
          const { data, ack } = this.mapPayload(payload, message);
          
          // CHANGED CODE ABOVE 🢁🢁🢁🢁🢁
          
          

          return transform(callback(data, ack)).pipe(
            filter((response: any) => !isNil(response)),
            map((response: any) => [response, ack]),
          );
        }),
        takeUntil(disconnect$),
      );
      source$.subscribe(([response, ack]) => {
        if (response.event) {
          return client.emit(response.event, response.data);
        }
        isFunction(ack) && ack(response);
      });
    });

then the mapPayload consumes it as such:

    if (!Array.isArray(payload)) {



       // CHANGED CODE BELOW 🢃🢃🢃🢃🢃
          
      if (message) {
        payload.event_name = message;
      }
      
       // CHANGED CODE ABOVE 🢁🢁🢁🢁🢁
       
       
       
      return { data: payload };
    }
    const lastElement = payload[payload.length - 1];
    const isAck = isFunction(lastElement);
    if (isAck) {
      const size = payload.length - 1;
      return {
        data: size === 1 ? payload[0] : payload.slice(0, size),
        ack: lastElement,
      };
    }



    // CHANGED CODE BELOW 🢃🢃🢃🢃🢃
    
    
    if(message) {
      payload.unshift({event_name: message})
    }
    
    
    // CHANGED CODE ABOVE 🢁🢁🢁🢁🢁
    
    
    
    return { data: payload };

The end result is, everytime I use the host to switchToWs().getData() -- I will also have the event_name property defined.

I thought about making a pull request to contribute this bit to the adapter in version 8.0.0 but I wanted to ask for some feedback on the use case itself and -- most importantly --- what would be the best case to propagate that configuration all the way down to the bindMessageHandlers in the adapter, as currently there's no constructor that accepts any kind of options there.

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 type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

1 participant