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

[Bug]: iOS Development build warnings/errors trigger SES error #7923

Closed
Gudahtt opened this issue Nov 23, 2023 · 1 comment · Fixed by #7924
Closed

[Bug]: iOS Development build warnings/errors trigger SES error #7923

Gudahtt opened this issue Nov 23, 2023 · 1 comment · Fixed by #7924
Labels
release-7.13.0 Issue or pull request that will be included in release 7.13.0 type-bug Something isn't working

Comments

@Gudahtt
Copy link
Member

Gudahtt commented Nov 23, 2023 •
edited
Loading

Describe the bug

Any warnings or errors in development builds are triggering an error from ses. The error is "Attempting to define a property on object that is not extensible", and it comes from a module in the react package that is only used for development builds. This error hides the underlying error/warning, making local development much more difficult.

Various development-build-only dependencies are not compatible with SES lockdown. On the MetaMask extension we worked around this by not calling lockdown in development builds. We should consider doing the same thing here.

Expected behavior

Errors and warnings should not trigger additional errors.

Screenshots/Recordings

Screenshot 2023-11-23 at 7 00 47 PM

Steps to reproduce

Follow the reproduction steps for this issue: #7920

Error messages or log output

 ERROR  TypeError: Attempting to define property on object that is not extensible.

This error is located at:
    in Unknown (created by SignatureRequest)
    in RCTView (created by View)
    in View (created by AnimatedComponent)
    ...

Version

main

Build type

None

Device

iPhone 13 Pro simulator

Operating system

iOS

Additional context

No response

Severity

No response
@Gudahtt Gudahtt added the type-bug Something isn't working label Nov 23, 2023
Gudahtt added a commit that referenced this issue Nov 23, 2023
@Gudahtt
Fix: Prevent SES errors in iOS dev builds
ea01697 
SES is now disabled in development builds. This was done as a
workaround to various incompatibilities between SES and React
development libraries, which were causing a SES error whenever a
warning or error was triggered in an iOS dev build.

Fixes #7923
@Gudahtt Gudahtt mentioned this issue Nov 23, 2023
Merged
13 tasks
Gudahtt added a commit that referenced this issue Nov 24, 2023
@Gudahtt
Fix: Prevent SES errors in iOS dev builds
50d7065 
SES is now disabled in development builds. This was done as a
workaround to various incompatibilities between SES and React
development libraries, which were causing a SES error whenever a
warning or error was triggered in an iOS dev build.

Fixes #7923
@leotm
Copy link
Member

leotm commented Nov 24, 2023

makes sense for now for a better devexp running our debug builds 👍
as you mentioned we've done on the MetaMask extension

Gudahtt added a commit that referenced this issue Nov 24, 2023
@Gudahtt
fix: Prevent SES errors in iOS dev builds (#7924)
0163b8b 
## **Description**

SES is now disabled in development builds. This was done as a workaround
to various incompatibilities between SES and React development
libraries, which were causing a SES error whenever a warning or error
was triggered in an iOS dev build.

## **Related issues**

Fixes #7923

## **Manual testing steps**

Follow the reproduction steps for this issue:
#7920

## **Screenshots/Recordings**

Before:

<img width="427" alt="Screenshot 2023-11-23 at 7 00 47 PM"
src="https://github.com/MetaMask/metamask-mobile/assets/2459287/11143d5c-f2ce-4594-a525-e74d52e96255">


After:

<img width="427" alt="Screenshot 2023-11-23 at 6 54 32 PM"
src="https://github.com/MetaMask/metamask-mobile/assets/2459287/3fd72105-28fc-4cdc-af50-61e9da3da780">

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [ ] I've included screenshots/recordings if applicable
- [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.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@metamaskbot metamaskbot added the release-7.13.0 Issue or pull request that will be included in release 7.13.0 label Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
release-7.13.0 Issue or pull request that will be included in release 7.13.0 type-bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Prevent SES errors in iOS dev builds MetaMask/metamask-mobile
3 participants
@leotm @Gudahtt @metamaskbot