[@sentry/node] Error URL doesn't include port number in Express

[DanielGibbsNZ]

Package + Version

• @sentry/node: 4.6.2

Description

The request URL (event.request.url) added by the Sentry.Handlers.requestHandler does not include the port. Looking at the source shows that it derives the URL from the hostname, which in Express is retrieved from req.hostname (which doesn't contain the port) rather than headers.host (which does).

This can be seen by running an Express server on a non-default port (e.g. 8080) and performing an HTTP request to https://example.com:8080/error which causes an error. The error shown in Sentry shows the URL as https://example.com/error.

[kamilogorek]

Feel free to introduce a PR for this change, will be more than happy to merge it :)

[DanielGibbsNZ]

Would it be an acceptable solution to always use req.headers.host for the hostname first and then fall back to the express and koa fields?

[kamilogorek]

Yes, as hostname is derived from it anyway – https://expressjs.com/en/api.html#req.hostname

Contains the hostname derived from the Host HTTP header.

However, we want to add port only if it's not a default (80 for https, 8000 for https).

[DanielGibbsNZ]

Regarding contributing: I forked the repository and followed the instructions in CONTRIBUTING.md but when I run yarn build in the packages/node directory I get a ton of TypeScript errors, most of which are something like Cannot find module '@sentry/types'. Is there some additional setup I need to do to be able to build the package?

[HazAT]

@DanielGibbsNZ Sorry, you need to run yarn build in the root once to have everything in place. Then yarn build in node should work.

[kamilogorek]

Closing the issue as a part of large repository cleanup, due to it being inactive and/or outdated.
Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it.
Cheers!