feat(core): add getUrl method to nest-applicaiton

[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
[x] 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?

Currently, there is no default behavior for consistently getting the server's url.
Issue Number: #3010

What is the new behavior?

After running await app.listen(<port>) you can add const url = await app.getUrl() and you will receive back a server address in the form of http(s)://[::1]:3000 or http(s)://127.0.0.1:3000 depending on if the server is inherently running on IPv6 or IPv4.

By default, Express runs on IPv6 and Fastify runs on IPv4, so both methods needed to be accounted for. The method needs to be async as it needs to wait to ensure that the server is listening before returning the url, otherwise the method for getting the server's address could return undefined.

This is the same implementation that Loopback uses (that @BrunnerLivio mentioned in the issue).

Does this PR introduce a breaking change?

[] Yes
[x] No

Other information

I couldn't seem to find where the tests were written for nest-application in the core package, but I will be happy to write some for this if need be. I tested this on my local machine with both Fastify and Express, and for both servers with using the default listen() function and with passing in an address to listen at. Please also let me know if you would like any documentation updates.

[coveralls]

Pull Request Test Coverage Report for Build 4647

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 19 unchanged lines in 5 files lost coverage.
• Overall coverage decreased (-0.08%) to 95.115%

---

Files with Coverage Reduction	New Missed Lines	%
packages/core/injector/container.ts	2	89.83%
packages/core/injector/instance-wrapper.ts	2	86.38%
packages/common/pipes/validation.pipe.ts	3	89.33%
packages/microservices/client/client-rmq.ts	4	87.63%
packages/core/injector/module.ts	8	86.89%

Totals
Change from base Build 3985:	-0.08%
Covered Lines:	4342
Relevant Lines:	4565

---

💛 - Coveralls

[BrunnerLivio]

I couldn't seem to find where the tests were written for nest-application in the core package

Currently there are no NestApplication unit and (direct) integration tests (yep, we should definitely add some).
I think most importantly for this feature we should add integration tests.
I'd suggest you add a new folder mkdir -p integration/nest-application/get-url and add your test-project in there.

In terms of documentation, I think none is needed. In the future this will be auto-documented by the API doc compiler from docs.nestjs.com :)

@kamilmysliwiec: Do you agree with that?

[BrunnerLivio]

If the server is not in listening-state it would not reject the promise. Could you add an error message, that the user should call .listen before calling .getUrl?

[jmcdo29]

I'll get the error message added in once I've got some more time to work on the functionality, great catch!

[jmcdo29]

I had to add in a boolean value to the NestApplication class to successfully determine in the app.listen method had been called. Express gives the HTTP server's _handle property a value but fastify doesn't. Otherwise, a reject is called and reason is given. I wasn't sure if you wanted it as a this.logger.error() and process.exit(1) or a reject so currently it logs and rejects. That can easily be updated.

[jmcdo29]

Currently there are no NestApplication unit and (direct) integration tests (yep, we should definitely add some).

Ah, okay, that definitely explains it.

I'd suggest you add a new folder mkdir -p integration/nest-application/get-url and add your test-project in there.

All right! I'll get that added in with the error message

In terms of documentation, I think none is needed. In the future this will be auto-documented by the API doc compiler from docs.nestjs.com :)

Cool! Was just following the bases of the Pull request template.

[jmcdo29]

I've added in tests under integration/nest-application/get-url like Livio suggested. Fastify by default works with IPv4, hence why there is not an IPv6 test for it, but there is for Express. Let me know if I can do anything else to enhance the PR. 👍

[BrunnerLivio]

LGTM. Well done @jmcdo29 :)

[jmcdo29]

@BrunnerLivio sorry, I didn't mean to re-request the review. Can't see a way to retract that

[kamilmysliwiec]

Thank you!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.