feat(core,websockets): use rxjs to check if values are observables

[micalevisk]

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?

Here's the full code: https://gitlab.com/micalevisk/nestjs-observable-issue

If we return a literal object that has the subscribe method on it, the application gets stuck

import { Controller, Get, Req } from '@nestjs/common'
import { of, Observable } from 'rxjs'

@Controller()
export class AppController {
  @Get()
  getHello(): any {
    return { subscribe(){} }
  }
}

If we return req.route (from Express), we got the following error (and 500 as a response)

Error: Route.subscribe() requires a callback function but got a [object Object]
    at Route.<computed> [as subscribe] (/x/node_modules/express/lib/router/route.js:202:15)
    at /x/node_modules/rxjs/src/internal/lastValueFrom.ts:59:12
    at new Promise (<anonymous>)
    at Object.lastValueFrom (/x/node_modules/rxjs/src/internal/lastValueFrom.ts:56:10)
    at RouterResponseController.transformToResult (/x/node_modules/@nestjs/core/router/router-response-controller.js:34:27)
    at /x/node_modules/@nestjs/core/router/router-execution-context.js:174:52
    at /x/node_modules/@nestjs/core/router/router-execution-context.js:47:19
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at /x/node_modules/@nestjs/core/router/router-proxy.js:9:17

import { Controller, Get, Req } from '@nestjs/common'
import { of, Observable } from 'rxjs'
import { Request } from 'express'

@Controller()
export class AppController {
  @Get()
  getHello(@Req() req: Request): string | Observable<string> | Record<string, any> {
    // return 'hello world' // Works
    // return of('hello world') // Works
    return req.route // Errors but should work too as it isn't an Observable
  }
}

What is the new behavior?

Instead of checking if the return of the method has the function subscribe, we're using isObservable from rxjs to check if the return is, in fact, a valid observable object

Thus, now with the above code the server will reply with this JSON instead:

{
  "path": "/",
  "stack": [
    {
      "name": "<anonymous>",
      "keys": [],
      "regexp": {
        "fast_star": false,
        "fast_slash": false
      },
      "method": "get"
    }
  ],
  "methods": {
    "get": true
  }
}

as expected.

And just {} when we return { subscribe(){} }

Does this PR introduce a breaking change?

• Yes
• No

Other information

There are other places in which isObservable is used. See:

While others are using isFunction(value.subscribe) and I'm unsure if this was intentional. See:

Should we do the same for

nest/packages/microservices/client/client-grpc.ts

Lines 211 to 212 in d9d628d

	const isUpstreamSubject =
	upstreamSubjectOrData && isFunction(upstreamSubjectOrData.subscribe);

nest/packages/microservices/client/client-grpc.ts

Lines 154 to 155 in d9d628d

	const isUpstreamSubject =
	upstreamSubjectOrData && isFunction(upstreamSubjectOrData.subscribe);

?

[coveralls]

Pull Request Test Coverage Report for Build dcbc02b5-7264-47ae-ad61-0d9f89c128bd

• 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.001%) to 94.043%

---

Totals
Change from base Build 0a7dce2a-d55f-4ed3-96fe-535d2466a616:	-0.001%
Covered Lines:	5762
Relevant Lines:	6127

---

💛 - Coveralls

[micalevisk]

looks like the CI pipeline is broken 🤔

[micalevisk]

btw the tests didn't cover the following

because
await Promise.resolve( Promise.resolve(123) ) return 123 (node16)

Should we drop that if?

https://github.com/micalevisk/nest/blob/88575ee0226b418491b4186fb9bc8943a6ee253b/packages/websockets/web-sockets-controller.ts#L169-L171