Skip to content

Do not show console error when libnames are failed to parse as a URL - #5993

Merged
canova merged 2 commits into
firefox-devtools:mainfrom
canova:url-parse-error
May 6, 2026
Merged

Do not show console error when libnames are failed to parse as a URL#5993
canova merged 2 commits into
firefox-devtools:mainfrom
canova:url-parse-error

Conversation

@canova

@canovacanova commented Apr 30, 2026

Copy link
Copy Markdown
Member

This is polluting the console output when we share a profile with URLs removed, because it will try to parse urls like http://<URL>:564:26.

Also ideally, we should start using pages array and ditch using the resource table.

Example profile:
Before / after

This is polluting the console output when we share a profile with URLs
removed, because it will try to parse urls like `http://<URL>:564:26`.
@canova
canova requested a review from fatadelApril 30, 2026 10:14
@codecov

codecovBot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.82%. Comparing base (61e5d10) to head (261969e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5993 +/- ##
=======================================
Coverage 83.81% 83.82% =======================================
Files 328 328 Lines 34255 34254 -1 Branches 9572 9574 +2 =======================================
Hits 28712 28712 + Misses 5115 5114 -1 
Partials 428 428 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fatadel

Copy link
Copy Markdown
Contributor

I was thinking maybe we could just lower the level instead of complete removing these messages? For example, make it console.debug. Wdyt?

@canova

Copy link
Copy Markdown
MemberAuthor

I was thinking maybe we could just lower the level instead of complete removing these messages? For example, make it console.debug. Wdyt?

I think the question comes down to why we want to show this console message. Previously this code had the assumption that all urls have to be valid urls. So if a url was failed to parse, then we would want to know that it's failed because it ideally shouldn't happen in real life. But that assumption has changed since we added the sanitization step that anonymizes the urls. Now it's possible to have urls that won't be parsed. I think console.debug won't add anything more than the noise. If we want to really debug this code path, it's still easy to do it with the debugger or by adding logs locally.

@drexelisus100-code

This comment was marked as spam.

@fatadel

Copy link
Copy Markdown
Contributor

I was thinking maybe we could just lower the level instead of complete removing these messages? For example, make it console.debug. Wdyt?

I think the question comes down to why we want to show this console message. Previously this code had the assumption that all urls have to be valid urls. So if a url was failed to parse, then we would want to know that it's failed because it ideally shouldn't happen in real life. But that assumption has changed since we added the sanitization step that anonymizes the urls. Now it's possible to have urls that won't be parsed. I think console.debug won't add anything more than the noise. If we want to really debug this code path, it's still easy to do it with the debugger or by adding logs locally.

I believe debug logs are suppressed in browser by default, so no real noise :) But any way works for me 👍🏻

@canova

Copy link
Copy Markdown
MemberAuthor

I believe debug logs are suppressed in browser by default, so no real noise :) But any way works for me 👍🏻

Ah I see. At least when I open a fresh Firefox DevTools, I see all of the console logs active:
Screenshot 2026-05-06 at 13 02 28

But that can be toggled off and maybe other browsers do it differently.

@canova
canova enabled auto-merge (squash) May 6, 2026 11:03
@canova
canova merged commit 8f49358 into firefox-devtools:mainMay 6, 2026
21 checks passed
@canovacanova mentioned this pull request May 7, 2026
canova added a commit that referenced this pull request May 7, 2026
Changes:
[Markus Stange] Use custom splitter component (#4606)
[fatadel] Fix Download button text color when clicked (#5985)
[Samuel Glauser] Fix fullscreen icon size in bottom box (#5987)
[Nazım Can Altınova] Add `profiler-cli` for querying profiles (#5963)
[Nazım Can Altınova] Bump profiler cli version to 0.1.0 (#5996)
[Markus Stange] Switch from max-height to maxHeight in JSX
style={{...}}. (#5990)
[carverdamien] Fix comment about how time and duration are stored
(#5997)
[Nazım Can Altınova] Do not show console error when libnames are failed
to parse as a URL (#5993)
[Nazım Can Altınova] Fix the unnecessary stringify of Uint8Array
contents during zip profile extraction (#6004)
And special thanks to our localizers:
en-CA: chutten
en-CA: Saurabh
en-GB: Ian Neal
es-CL: ravmn
fy-NL: Fjoerfoks
ia: Melo46
nl: Mark Heijl
ru: Valery Ledovskoy
sv-SE: Andreas Pettersson
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@canova@fatadel@drexelisus100-code