Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Sentry] 'View: Root TypeError: Invalid URL:null' instead of displaying the expected 'Something went wrong' window for incorrect links inserted in MetaMask's in-build browser. #11479

Closed
ZbrancaI opened this issue Sep 27, 2024 · 5 comments · Fixed by #11581
Closed

[Sentry] 'View: Root TypeError: Invalid URL:null' instead of displaying the expected 'Something went wrong' window for incorrect links inserted in MetaMask's in-build browser. #11479

ZbrancaI opened this issue Sep 27, 2024 · 5 comments · Fixed by #11581
Assignees
@frankvonhoven
Labels
area-Sentry Issue from Sentry INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. regression-prod-7.35.0 Regression bug that was found in production in release 7.35.0 regression-prod-7.35.1 Regression bug that was found in production in release 7.35.1 regression-RC-7.32.0 Regression bug that was found in release candidate (RC) for release 7.32.0 release-7.33.0 Issue or pull request that will be included in release 7.33.0 release-blocker This bug is blocking the next release Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-mobile-platform type-bug Something isn't working

Comments

@ZbrancaI
Copy link 

          Entering an incorrect URL triggered the error 'View: Root TypeError: Invalid URL:null' instead of displaying the expected 'Something went wrong' window. This issue seems to occur specifically when there's a mistake in the protocol section ('https//:'). If the error is made elsewhere in the URL, the proper error window appears.

Steps to reproduce:

  1. Open the wallet;
  2. Open the in-build browser of the wallet;
  3. Insert the wrong URL with the mention made in the description (ex: 'htpps://app.uniswap.org');
  4. The error 'View: Root TypeError: Invalid URL:null' will occur instead of the 'Something went wrong' window.
  • Build: 1437
  • Version: 7.32.0
  • OS: Android
  • Device: Xiaomi Redmi Note 8 Pro

Link to the recording & screenshot:

https://drive.google.com/file/d/1oLiGLQ3XNFMJ9PDjI8A-qJ71uUm1Atet/view?usp=sharing
https://drive.google.com/file/d/1JraUa69u6ZLNFE6Lzb2Xk7qbq6JOkzHz/view?usp=sharing

Expected behavior:

  • Entering an incorrect URL, including errors in the protocol section, should trigger the 'Something went wrong' window instead of the 'View: Root TypeError: Invalid URL:null' error.

Actual behavior:

  • Entering an incorrect URL, with mistakes on the protocol section, triggers the 'View: Root TypeError: Invalid URL:null'.

Workaround for the error:

  • Cleaning the browser history from the Security & Privacy setting.

Originally posted by @ZbrancaI in https://github.com/MetaMask/mobile-planning/issues/1929#issuecomment-2378879941
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Sep 27, 2024
@anaamolnar anaamolnar added regression-RC-7.32.0 Regression bug that was found in release candidate (RC) for release 7.32.0 team-wallet-ux labels Sep 27, 2024
@Andepande Andepande added the type-bug Something isn't working label Sep 27, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Sep 27, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Sep 27, 2024
@Andepande Andepande added Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing and removed team-wallet-ux labels Sep 27, 2024
@Cal-L Cal-L added the release-blocker This bug is blocking the next release label Sep 30, 2024
@frankvonhoven frankvonhoven mentioned this issue Oct 1, 2024
Closed
7 tasks
@Cal-L Cal-L mentioned this issue Oct 2, 2024
Merged
7 tasks
github-merge-queue bot pushed a commit that referenced this issue Oct 2, 2024
@Cal-L
fix: Fix invalid browser url crash (#11581)
e80a1e8 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes the crash in 7.32.0 for invalid URL in browser. The global
`URL` didn't catch invalid urls gracefully, so we are using the lib
`url-parser`

## **Related issues**

Fixes: #11479 

## **Manual testing steps**

1. Go to browser
2. Enter `httttps://app.uniswap.org`
3. Website shows invalid url scheme
4. App doesn't crash

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/2980c632-ed4b-48a4-976c-b1da54c590ce


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Oct 2, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Oct 2, 2024
runway-github bot added a commit that referenced this issue Oct 2, 2024
@runway-github
temp+fix: Fix invalid browser url crash (#11581)
5d5396f 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes the crash in 7.32.0 for invalid URL in browser. The global
`URL` didn't catch invalid urls gracefully, so we are using the lib
`url-parser`

## **Related issues**

Fixes: #11479 

## **Manual testing steps**

1. Go to browser
2. Enter `httttps://app.uniswap.org`
3. Website shows invalid url scheme
4. App doesn't crash

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/2980c632-ed4b-48a4-976c-b1da54c590ce


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
runway-github bot added a commit that referenced this issue Oct 2, 2024
@runway-github
temp+fix: Fix invalid browser url crash (#11581)
c3505bf 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes the crash in 7.32.0 for invalid URL in browser. The global
`URL` didn't catch invalid urls gracefully, so we are using the lib
`url-parser`

## **Related issues**

Fixes: #11479

## **Manual testing steps**

1. Go to browser
2. Enter `httttps://app.uniswap.org`
3. Website shows invalid url scheme
4. App doesn't crash

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

https://github.com/user-attachments/assets/2980c632-ed4b-48a4-976c-b1da54c590ce

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
runway-github bot pushed a commit that referenced this issue Oct 2, 2024
@Cal-L
chore(runway): cherry-pick fix: Fix invalid browser url crash (#11581)
79226aa 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes the crash in 7.32.0 for invalid URL in browser. The global
`URL` didn't catch invalid urls gracefully, so we are using the lib
`url-parser`

## **Related issues**

Fixes: #11479 

## **Manual testing steps**

1. Go to browser
2. Enter `httttps://app.uniswap.org`
3. Website shows invalid url scheme
4. App doesn't crash

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/2980c632-ed4b-48a4-976c-b1da54c590ce


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
@runway-github runway-github bot mentioned this issue Oct 2, 2024
Merged
7 tasks
frankvonhoven pushed a commit that referenced this issue Oct 2, 2024
@runway-github @Cal-L
chore(runway): cherry-pick fix: Fix invalid browser url crash (#11583)
3337a75 
- fix: Fix invalid browser url crash (#11581)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes the crash in 7.32.0 for invalid URL in browser. The global
`URL` didn't catch invalid urls gracefully, so we are using the lib
`url-parser`

## **Related issues**

Fixes: #11479 

## **Manual testing steps**

1. Go to browser
2. Enter `httttps://app.uniswap.org`
3. Website shows invalid url scheme
4. App doesn't crash

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->



https://github.com/user-attachments/assets/2980c632-ed4b-48a4-976c-b1da54c590ce


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding

Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling

guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
[e80a1e8](e80a1e8)

Co-authored-by: Cal Leung <cal.leung@consensys.net>
@gauthierpetetin gauthierpetetin added the release-7.33.0 Issue or pull request that will be included in release 7.33.0 label Oct 10, 2024
@sentry-io Sentry.io
Copy link

sentry-io bot commented Nov 21, 2024

Sentry Issue: METAMASK-MOBILE-349G

@metamaskbot metamaskbot changed the title 'View: Root TypeError: Invalid URL:null' instead of displaying the expected 'Something went wrong' window for incorrect links inserted in MetaMask's in-build browser. [Sentry] 'View: Root TypeError: Invalid URL:null' instead of displaying the expected 'Something went wrong' window for incorrect links inserted in MetaMask's in-build browser. Nov 22, 2024
@metamaskbot metamaskbot added area-Sentry Issue from Sentry regression-prod-7.35.0 Regression bug that was found in production in release 7.35.0 labels Nov 22, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Nov 22, 2024
@smilingkylan smilingkylan mentioned this issue Nov 29, 2024
Open
@sentry-io Sentry.io
Copy link

sentry-io bot commented Nov 29, 2024

Sentry Issue: METAMASK-MOBILE-351W

@sentry-io Sentry.io
Copy link

sentry-io bot commented Nov 29, 2024

Sentry Issue: METAMASK-MOBILE-357R

@sentry-io Sentry.io
Copy link

sentry-io bot commented Nov 29, 2024

Sentry Issue: METAMASK-MOBILE-350J

@sentry-io Sentry.io
Copy link

sentry-io bot commented Dec 3, 2024

Sentry Issue: METAMASK-MOBILE-355T

@metamaskbot metamaskbot added the regression-prod-7.35.1 Regression bug that was found in production in release 7.35.1 label Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@frankvonhoven frankvonhoven

Labels
area-Sentry Issue from Sentry INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. regression-prod-7.35.0 Regression bug that was found in production in release 7.35.0 regression-prod-7.35.1 Regression bug that was found in production in release 7.35.1 regression-RC-7.32.0 Regression bug that was found in release candidate (RC) for release 7.32.0 release-7.33.0 Issue or pull request that will be included in release 7.33.0 release-blocker This bug is blocking the next release Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-mobile-platform type-bug Something isn't working
Projects
Bugs by severity
Archived in project
Milestone
No milestone
7 participants
@gauthierpetetin @Cal-L @metamaskbot @anaamolnar @ZbrancaI @Andepande @frankvonhoven