doc: mention the behaviour if URL is invalid #2966
Conversation
thefourtheye
added
http
What?? Are you telling me that if something listens on |
|
@ChALkeR 👍 for fixing it. |
|
+1 for fixing |
|
@targos Documenting it is the opposite to fixing it. I treat this as a bug, the behavior is completely unexpected. Everything except for the |
|
If #2967 only goes on master, we can land this change on without the note and add the note in v4.x with a separate PR |
thefourtheye
force-pushed
the
http-request-doc-improvement
branch
from
b659563
to
281b254
Compare
|
@ChALkeR I updater the PR as per your comment here #2967 (comment). PTAL. |
If the URL passed to `http{s}.request` or `http{s}.get` is not properly
parsable by `url.parse`, we fall back to use `localhost` and port 80.
This creates confusing error messages like in this question
http://stackoverflow.com/q/32675907/1903116.
This patch throws an error message, if `url.parse` fails to parse the
URL properly.
Previous Discussion: nodejs#2966
PR-URL: nodejs#2967
|
Bump! |
|
Okay tagging this as lts-watch as this would be important to have in a LTS release. cc @nodejs/tsc |
If the URL passed to `http{s}.request` or `http{s}.get` is not properly
parsable by `url.parse`, we fall back to use `localhost` and port 80.
This creates confusing error messages like in this question
http://stackoverflow.com/q/32675907/1903116.
This patch throws an error message, if `url.parse` fails to parse the
URL properly.
Previous Discussion: #2966
PR-URL: #2967
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
|
Recommend landing this documentation change in v4.x (via v4.x-staging) and documenting the bug in the known issues in the release |
|
@jasnell Cool. Do the changes look fine to you? |
|
The specific edits that I see here, yes, but I notice that the documentation updates do not mention the errant fallback behavior. I'm wondering if we shouldn't go ahead and add it to the docs but couch it in language like "Currently..." and "this will be fixed soon", that sort of thing. |
If the URL passed to `http.request` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116.
thefourtheye
force-pushed
the
http-request-doc-improvement
branch
from
281b254
to
4e0904e
Compare
|
@jasnell Okay, I included "This will be fixed soon" in the Notes section. Is that what you meant? |
|
@thefourtheye ... yep, that works. I would also list this in the known issues section in the release notes |
|
LGTM |
If the URL passed to `http.request` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116. PR-URL: #2966 Reviewed-By: James M Snell <[email protected]>
|
Thanks for the review. Landed at 14135f0 |
|
Sorry. This change was not supposed to go to |
If the URL passed to `http.request` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116. PR-URL: #2966 Reviewed-By: James M Snell <[email protected]>
|
Okay, landed the change in |
If the URL passed to `http{s}.request` or `http{s}.get` is not properly
parsable by `url.parse`, we fall back to use `localhost` and port 80.
This creates confusing error messages like in this question
http://stackoverflow.com/q/32675907/1903116.
This patch throws an error message, if `url.parse` fails to parse the
URL properly.
Previous Discussion: nodejs#2966
PR-URL: nodejs#2967
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
If the URL passed to `http.request` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116. PR-URL: #2966 Reviewed-By: James M Snell <[email protected]>
If the URL passed to
http.requestis not properly parsable byurl.parse, wefall back to use
localhostand port 80. This creates confusing error messageslike in this question http://stackoverflow.com/q/32675907/1903116.