Fix expo issue [#17903](https://github.com/expo/expo/issues/17903) #841
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Need it soon :) |
|
Please review this PR, we also need it |
|
Also needed |
Codecov ReportBase: 79.73% // Head: 79.73% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #841 +/- ##
==========================================
- Coverage 79.73% 79.73% -0.01%
==========================================
Files 210 210
Lines 10671 10673 +2
Branches 2584 2585 +1
==========================================
+ Hits 8509 8510 +1
- Misses 2162 2163 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
On closer investigation, Metro already makes an attempt to check for known URLs here but it was buggy - #910. |
facebook-github-bot
pushed a commit
that referenced
this pull request
Jan 3, 2023
Summary: As reported in expo/expo#17903, Metro logs errors to console attempting to read nonsense file paths. This occurs when trying to build a code frame for a URL `frame.file`, which Metro *should* skip attempting for any URLs it has already seen. Typically, URLs reach this point when a particular frame cannot be symbolicated. The problem goes back to D25262626 (8cf2434), which introduced resolving the "file" to an absolute path before checking for its presence in the list of known URLs. The `urls.has` check has effectively been broken since then. This restores the `urls` map lookup with the "unresolved" `file`, and allows `getCodeFrame` to skip trying to read the nonsense path. The runtime result is the same either way - `getCodeFrame` moves on to the next frame in the stack, but no error should be logged to console. This also eliminates some noisy logging to console while running Jest tests. Supersedes #841 Changelog: [Fix] Don't log ENOENT errors to console for expected URL stack frames Pull Request resolved: #910 Test Plan: ### Before Added failing tests to `Server-test.js`: ``` yarn jest Server/__tests__/Server-test yarn run v1.22.19 $ /Users/robhogan/gh/metro-symbolicate/node_modules/.bin/jest Server/__tests__/Server-test (node:5886) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 unhandledRejection listeners added to [process]. Use emitter.setMaxListeners() to increase limit (Use `node --trace-warnings ...` to show where the warning was created) FAIL packages/metro/src/Server/__tests__/Server-test.js (6.25 s) processRequest ✓ returns JS bundle source on request of *.bundle (978 ms) ✓ returns a bytecode bundle source on request of *.bundle?runtimeBytecodeVersion (209 ms) ✓ returns JS bundle without the initial require() call (106 ms) ✓ returns Last-Modified header on request of *.bundle (104 ms) ✓ returns build info headers on request of *.bundle (108 ms) ✓ returns Content-Length header on request of *.bundle (99 ms) ✓ returns 404 on request of *.bundle when resource does not exist (103 ms) ✓ returns 304 on request of *.bundle when if-modified-since equals Last-Modified (110 ms) ✓ returns 200 on request of *.bundle when something changes (ignoring if-modified-since headers) (114 ms) ✓ supports the `modulesOnly` option (100 ms) ✓ supports the `shallow` option (99 ms) ✓ should handle DELETE requests on *.bundle (112 ms) ✓ multiple DELETE requests on *.bundle succeed (109 ms) ✓ DELETE succeeds with a nonexistent path (102 ms) ✓ DELETE handles errors (106 ms) ✓ returns sourcemap on request of *.map (109 ms) ✓ source map request respects `modulesOnly` option (113 ms) ✓ does not rebuild the graph when requesting the sourcemaps after having requested the same bundle (101 ms) ✓ does build a delta when requesting the sourcemaps after having requested the same bundle (103 ms) ✓ does rebuild the graph when requesting the sourcemaps if the bundle has not been built yet (114 ms) ✓ passes in the platform param (116 ms) ✓ passes in the unstable_transformProfile param (101 ms) ✓ rewrites URLs before bundling (103 ms) ✓ does not rebuild the bundle when making concurrent requests (108 ms) /assets endpoint ✓ should serve simple case (88 ms) ✓ should parse the platform option (77 ms) ✓ should serve range request (75 ms) ✓ should serve assets files's name contain non-latin letter (73 ms) ✓ should use unstable_path if provided (80 ms) ✓ should parse the platform option if tacked onto unstable_path (82 ms) ✓ unstable_path can escape from projectRoot (74 ms) build(options) ✓ Calls the delta bundler with the correct args (82 ms) /symbolicate endpoint ✓ should symbolicate given stack trace (105 ms) ✓ should update the graph when symbolicating a second time (114 ms) ✓ supports the `modulesOnly` option (137 ms) ✓ supports the `shallow` option (108 ms) ✓ should symbolicate function name if available (105 ms) ✓ should collapse frames as specified in customizeFrame (107 ms) ✕ should leave original file and position when cannot symbolicate (132 ms) should rewrite URLs before symbolicating ✓ mapped location symbolicates correctly (108 ms) ✕ unmapped location returns the rewritten URL (112 ms) /symbolicate handles errors ✓ should symbolicate given stack trace (80 ms) ● processRequest › /symbolicate endpoint › should rewrite URLs before symbolicating › unmapped location returns the rewritten URL expect(jest.fn()).not.toHaveBeenCalled() Expected number of calls: 0 Received number of calls: 1 1: [Error: ENOENT: `/root/http:/localhost:8081/mybundle.bundle?runModule=true&TEST_URL_WAS_REWRITTEN=true`: no such file or directory] 119 | afterEach(() => { 120 | expect(mockConsoleWarn).not.toHaveBeenCalled(); > 121 | expect(mockConsoleError).not.toHaveBeenCalled(); | ^ 122 | }); 123 | 124 | let server; at Object.toHaveBeenCalled (packages/metro/src/Server/__tests__/Server-test.js:121:34) ● processRequest › /symbolicate endpoint › should leave original file and position when cannot symbolicate expect(jest.fn()).not.toHaveBeenCalled() Expected number of calls: 0 Received number of calls: 1 1: [Error: ENOENT: `/root/http:/localhost:8081/mybundle.bundle?runModule=true`: no such file or directory] 119 | afterEach(() => { 120 | expect(mockConsoleWarn).not.toHaveBeenCalled(); > 121 | expect(mockConsoleError).not.toHaveBeenCalled(); | ^ 122 | }); 123 | 124 | let server; at Object.toHaveBeenCalled (packages/metro/src/Server/__tests__/Server-test.js:121:34) Test Suites: 1 failed, 1 total Tests: 2 failed, 40 passed, 42 total Snapshots: 3 passed, 3 total Time: 6.383 s, estimated 7 s ``` ### After ``` yarn jest Server/__tests__/Server-test yarn run v1.22.19 $ /Users/robhogan/gh/metro-symbolicate/node_modules/.bin/jest Server/__tests__/Server-test (node:6573) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 unhandledRejection listeners added to [process]. Use emitter.setMaxListeners() to increase limit (Use `node --trace-warnings ...` to show where the warning was created) PASS packages/metro/src/Server/__tests__/Server-test.js (5.609 s) processRequest ✓ returns JS bundle source on request of *.bundle (683 ms) ✓ returns a bytecode bundle source on request of *.bundle?runtimeBytecodeVersion (217 ms) ✓ returns JS bundle without the initial require() call (107 ms) ✓ returns Last-Modified header on request of *.bundle (103 ms) ✓ returns build info headers on request of *.bundle (108 ms) ✓ returns Content-Length header on request of *.bundle (101 ms) ✓ returns 404 on request of *.bundle when resource does not exist (99 ms) ✓ returns 304 on request of *.bundle when if-modified-since equals Last-Modified (108 ms) ✓ returns 200 on request of *.bundle when something changes (ignoring if-modified-since headers) (112 ms) ✓ supports the `modulesOnly` option (101 ms) ✓ supports the `shallow` option (104 ms) ✓ should handle DELETE requests on *.bundle (112 ms) ✓ multiple DELETE requests on *.bundle succeed (114 ms) ✓ DELETE succeeds with a nonexistent path (92 ms) ✓ DELETE handles errors (113 ms) ✓ returns sourcemap on request of *.map (106 ms) ✓ source map request respects `modulesOnly` option (109 ms) ✓ does not rebuild the graph when requesting the sourcemaps after having requested the same bundle (106 ms) ✓ does build a delta when requesting the sourcemaps after having requested the same bundle (101 ms) ✓ does rebuild the graph when requesting the sourcemaps if the bundle has not been built yet (113 ms) ✓ passes in the platform param (108 ms) ✓ passes in the unstable_transformProfile param (100 ms) ✓ rewrites URLs before bundling (100 ms) ✓ does not rebuild the bundle when making concurrent requests (110 ms) /assets endpoint ✓ should serve simple case (95 ms) ✓ should parse the platform option (73 ms) ✓ should serve range request (73 ms) ✓ should serve assets files's name contain non-latin letter (77 ms) ✓ should use unstable_path if provided (76 ms) ✓ should parse the platform option if tacked onto unstable_path (74 ms) ✓ unstable_path can escape from projectRoot (80 ms) build(options) ✓ Calls the delta bundler with the correct args (81 ms) /symbolicate endpoint ✓ should symbolicate given stack trace (100 ms) ✓ should update the graph when symbolicating a second time (109 ms) ✓ supports the `modulesOnly` option (123 ms) ✓ supports the `shallow` option (102 ms) ✓ should symbolicate function name if available (101 ms) ✓ should collapse frames as specified in customizeFrame (99 ms) ✓ should leave original file and position when cannot symbolicate (131 ms) should rewrite URLs before symbolicating ✓ mapped location symbolicates correctly (106 ms) ✓ unmapped location returns the rewritten URL (103 ms) /symbolicate handles errors ✓ should symbolicate given stack trace (76 ms) Test Suites: 1 passed, 1 total Tests: 42 passed, 42 total Snapshots: 3 passed, 3 total Time: 5.7 s, estimated 7 s ``` Reviewed By: motiz88, huntie Differential Revision: D42246439 Pulled By: robhogan fbshipit-source-id: 76019bf48588e82c4091abc5bc7db8c88098c48c
|
Thanks again @gavin1995, closing as this is superseded by #910, now released in 0.74.0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fix expo issue #17903
Test plan
dependencies
{
"expo": "45.0.5",
"expo-location": "~14.2.2",
"expo-navigation-bar": "~1.2.0",
"expo-splash-screen": "~0.15.1",
"expo-status-bar": "~1.3.0",
"expo-updates": "~0.13.2",
"react-native": "0.68.2"
}
issue
The file in the _symbolicate method may be a url link.