url: improve invalid url performance#49692
Conversation
|
Review requested:
|
0501c68 to
79415ed
Compare
|
@nodejs/tsc I've removed the Safari returns: None of them have the |
|
I would keep the input, and even add the base, it can very hard to understand what is the error without seeing the input and base |
|
Could there be a way to get the same performance improvements without removing helpful debugging information? If not, maybe we could use a flag to opt-in to the slower option that outputs the |
Probably, although I'm not sure. This requires a similar approach to what |
|
FWIW, I tend to agree with @aduh95 that removing error information for the sake of performance (on an error path!) does not benefit users. I thought that |
|
How much is actually the performance hit when keeping the input as an attribute? |
BridgeAR
left a comment
There was a problem hiding this comment.
I do not think it's a good idea to remove important debug information in favor of performance. We can definitely improve the performance but debug information is more important. I had an open PR to improve the situation a lot and I have to get back to that again.
147dd75 to
a9c408a
Compare
a9c408a to
29267d9
Compare
29267d9 to
c59b136
Compare
|
Hey @BridgeAR, can you re-review this pull request, and remove your block? |
|
I suspect he would still object since the URL isn't part of the message string, what's the cost of adding that back? |
I don't follow. URL was never part of the message string? Input is a property of the error object. On top of that I've also added base property to the error as well. |
|
@anonrig ah, I'm probably mixing it up with another PR then, lgtm |
|
Adding to @nodejs/tsc agenda to resolve the block. |
|
There's no need for the TSC to intervene if you addressed the blocking comment |
|
Landed in c829c03 |
PR-URL: #49692 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: #49692 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs#49692 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs#49692 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Some easy wins for
Invalid URLpath.cc @nodejs/url @nodejs/performance