feat: create method on client to get websocket pattern

[jmcdo29]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior?

The only way to get the websocket pattern is through reflection, which is unavailable inside of filters as the ArgumentsHost doesn't have a getClass or getHandler method.

Issue Number: #10520

What is the new behavior?

ExecutionContext#switchToWs().getClient() now has a getPattern() method attached to it so that the pattern can be retrieved in filters.

This pattern is still a reflected pattern, and not retrieved from the direct request, as we don't, to my knowledge, bind the exception filter for when invalid patterns are called (if I'm wrong about that please point to where it is)

Does this PR introduce a breaking change?

• Yes
• No

Other information

[coveralls]

Pull Request Test Coverage Report for Build 687f5680-c7ea-440a-8484-35693509459c

• 3 of 4 (75.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.01%) to 92.951%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/core/helpers/execution-context-host.ts	0	1	0.0%

Totals
Change from base Build 13b747f0-1270-494c-96aa-f7166e331f9e:	-0.01%
Covered Lines:	6382
Relevant Lines:	6866

---

💛 - Coveralls

[kamilmysliwiec]

ExecutionContext#switchToWs().getClient() returns the Socket instance. What if someone used this type (as socket.io now ships TS declarations out-of-the-box) as a type argument, as follows:

ExecutionContext#switchToWs().getClient<Socket>()

?

[jmcdo29]

ExecutionContext#switchToWs().getClient() returns the Socket instance. What if someone used this type (as socket.io now ships TS declarations out-of-the-box) as a type argument, as follows:

ExecutionContext#switchToWs().getClient<Socket>()

?

We could update the generic of getClient() to be getClient<T>(): T & { getPattern: () => string }. Might not be the most ideal, but with the current approach I think it's the best we've got

[kamilmysliwiec]

Suggested change

	Object.assign(args[0] ?? {}, {
	getPattern: () => this.reflectCallbackPattern(callback),
	});
	Object.assign(args[0] ?? {}, {
	getPattern: () => this.reflectCallbackPattern(callback),
	});

Hmm.. I'm wondering if this won't cause some unexpected issues 🤔 If socket represents a single connection (and so its instance object is shared?), there's a probability that 2 messages from that socket might be processed asynchronously (independently).

Example:

• Connection to server is established (socket instance is created)
• Message A is emitted
  • we mutate the "socket instance" enhancing it with the "getPattern()" method
  • before this message is processed, some asynchronous operation is triggered in (guard/interceptor/wherever)
• In the meantime, message B is emitted
  • we mutate the same "socket instance" again replacing the previous "getPattern()" method
• Async operation (mentioned above) completes and we're back to processing message A.
• In another interceptor/guard we call client.getPattern() when processing message A but it's already overridden with the message B's "getPattern" implementation

[jmcdo29]

Rather than mutating the client, could I add a new args entry and let getPattern() retrieve args[2]? Would that be more "stable"?

[jmcdo29]

Great call by the way! I moved getPattern to be a method on WsArgumentHost instead of under the getClient() so that it should be unique per request as the WsArgumentHost already is. Tests are still passing and the interfaces are updated. Let me know if you think of any other issues with this approach 😸

[jmcdo29]

npm v9 was released on 2022-10-24 is is not compatible with node 12. Today (2022-11-09) it was set to the @latest tag for npm so our CircleCI builds started breaking.

Wednesday Nov. 9th (General Availability)
To ensure npm@9.x is considered "non-breaking" for Node.js LTS we will codify a set of exit criteria in collaboration with the Release WG
npm@9.x will be set to the latest dist-tag (becoming the latest, maintained version of npm)
A PR will be opened to land npm@9.x in nodejs/node's main branch (exposing experimental/nightly users to this latest version)

https://github.blog/changelog/2022-10-24-npm-v9-0-0-released/

[jmcdo29]

Hey @kamilmysliwiec anything else you'd like me to work with on this feature?

[jmcdo29]

Hey @kamilmysliwiec any chance this can make it in before v10?

[jmcdo29]

@kamilmysliwiec just making sure there's visibility on this. I know you've got your hands full with a lot.

I'd like to get this merged in so I can make a minor release with a new feature for ogma before I release a major upgrade with breaking changes, as this would be a super helpful feature when it comes to logging errors

[kamilmysliwiec]

LGTM