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

Always loading and report error "Your message must be < 32kb." in console #25244

Closed
9 tasks
ZhiGuang opened this issue Jun 12, 2024 · 5 comments
Closed
9 tasks

Always loading and report error "Your message must be < 32kb." in console #25244

ZhiGuang opened this issue Jun 12, 2024 · 5 comments
Labels
external-contributor Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-extension-platform Extension Platform team type-bug Something isn't working

Comments

@ZhiGuang
Copy link

What is this about?

Metamask has been stuck on the loading page for a long time.

Browser version: Chrome 126.0.6478.57
Metamask version: 11.16.9

All software is the latest version.

My metamask has been used for a long time and there are dozens of accounts in it.

WX20240612-144155@2x

Scenario

No response

Design

No response

Technical Details

No response

Threat Modeling Framework

No response

Acceptance Criteria

No response

Stakeholder review needed before the work gets merged

  • Engineering (needed in most cases)
  • Design
  • Product
  • QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
  • Security
  • Legal
  • Marketing
  • Management (please specify)
  • Other (please specify)

References

No response
@danjm
Copy link
Contributor

danjm commented Jun 12, 2024

thanks @ZhiGuang, we are investigating this

Gudahtt added a commit that referenced this issue Jun 12, 2024
@Gudahtt
fix: Capture Segment errors during initialization
ac91516 
We were submitting Segment events in the MetaMetrics constructor
without cathing errors. This was the sole place where we failed to
catch errors resulting from Segment calls.

Fixes #25244
@Gudahtt Gudahtt mentioned this issue Jun 12, 2024
Merged
7 tasks
Gudahtt added a commit that referenced this issue Jun 12, 2024
@Gudahtt
fix: Capture Segment errors during initialization
962f852 
We were submitting Segment events in the MetaMetrics constructor
without cathing errors. This was the sole place where we failed to
catch errors resulting from Segment calls.

Fixes #25244
Gudahtt added a commit that referenced this issue Jun 12, 2024
@Gudahtt
fix: Capture Segment errors during initialization
46a141e 
We were submitting Segment events in the MetaMetrics constructor
without cathing errors. This was the sole place where we failed to
catch errors resulting from Segment calls.

Fixes #25244
@Gudahtt Gudahtt added type-bug Something isn't working Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. labels Jun 12, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Jun 12, 2024
@Gudahtt Gudahtt added the team-extension-platform Extension Platform team label Jun 12, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Jun 12, 2024
@Gudahtt
Copy link
Member

Gudahtt commented Jun 12, 2024

We are still having trouble reproducing this, but we believe we have a fix that will enable MetaMask to continue past this error at least. We will release this as soon as we can

Gudahtt added a commit that referenced this issue Jun 12, 2024
@Gudahtt
fix: Capture Segment errors during initialization (#25253)
32af889 
<!--
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**

We were submitting Segment events in the MetaMetrics constructor without
cathing errors. This was the sole place where we failed to catch errors
resulting from Segment calls.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25253?quickstart=1)

## **Related issues**

Mitigates #25244

## **Manual testing steps**

Sure of exact reproduction steps at this time.

## **Screenshots/Recordings**

N./A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [ ] 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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] 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.
k-g-j pushed a commit that referenced this issue Jun 12, 2024
@Gudahtt @k-g-j
fix: Capture Segment errors during initialization (#25253)
e61821b 
<!--
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**

We were submitting Segment events in the MetaMetrics constructor without
cathing errors. This was the sole place where we failed to catch errors
resulting from Segment calls.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25253?quickstart=1)

## **Related issues**

Mitigates #25244

## **Manual testing steps**

Sure of exact reproduction steps at this time.

## **Screenshots/Recordings**

N./A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [ ] 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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] 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.
@ZhiGuang
Copy link
Author

Wow, it seems to have been solved on 11.16.10. Thanks very much and good job.

WX20240613-232302@2x

@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Jun 13, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Jun 13, 2024
@benjisclowder
Copy link
Contributor

Collaborative Effort Required for Root Cause Analysis (RCA) on Critical Issues

We are quickly approaching the end of the quarter and we encourage you once again to take some moments and perform RCA on this critical issue. You may do so by answering the questions below:

What PR fixed the issue?
Can you pinpoint the commit from which the issue originated?
Write a short explanation of the technical cause of the bug
How could we have avoided merging this bug? What would have had to be different about our code, tests or processes?
4.1. Were there any missing unit, e2e or manual tests that could have preempted this issue?
4.2. Were there any other elements lacking, such as typed code, comprehensive documentation, well-architected APIs, etc., that might have prevented this issue?
4.3. If your answer to a and b is no, then is there anything at all that you can think of that, if it had been different before this bug was introduced, would have prevented it from being merged?
Please provide your answers as a reply to this comment and tag me as well.

You can read more about the initiative here. Thank you!

Tagging eng. who added the fix: @Gudahtt

@Gudahtt
Copy link
Member

Gudahtt commented Jul 15, 2024

@benjisclowder

  • What PR fixed the issue?
    There were two problems here. First, that we crashed if the call to Segment failed. Second, that the call to Segment was failing.
    We fixed the first problem in fix: Capture Segment errors during initialization #25253. We did not discover the cause of the second problem, and it might still be outstanding. I will focus on the first problem in this RCA, the second one will show up again in Sentry and can be dealt with separately that way.

  • Can you pinpoint the commit from which the issue originated?
    Persisting segment events in MetaMetricsController store #16198

  • Write a short explanation of the technical cause of the bug
    This was caused by poor error handling. This operation should be expected to fail (because it's making a network requests), but we were not handling this failure.

  • How could we have avoided merging this bug? What would have had to be different about our code, tests or processes?
    Here are two ideas:

    • Error handling documentation. We don't have any documented recommendations for error handling.
    • Better unit test coverage

The scenario in this case was an edge case that we probably would not expect to be covered by e2e tests or manual testing, I don't think this points to any major gaps there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
external-contributor Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-extension-platform Extension Platform team type-bug Something isn't working
Projects
Bugs by severity
Archived in project
Milestone
No milestone
Development

No branches or pull requests
5 participants
@ZhiGuang @Gudahtt @danjm @metamaskbot @benjisclowder