Fix ENOENT during symbolication for known source URLs

[robhogan]

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 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.

Supersedes #841

Changelog: [Fix] Don't log ENOENT errors to console for expected URL stack frames

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

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@robhogan merged this pull request in 1031ae6.