Skip to content

Fix symbolication of stack traces with Windows absolute paths - #1939

Open
robhogan wants to merge 1 commit into
mainfrom
pr1939
Open

robhogan wants to merge 1 commit into
mainfrom
pr1939

Conversation

@robhogan

@robhogan robhogan commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

metro-symbolicate doesn't allow : in file names when parsing stack frames, so a Windows absolute path is split at the drive letter. someFunc@D:\app\foo.js:4:0 is read as function D in file \app\foo.js. The output then has a stray someFunc@ and a file name with no drive, which breaks finding the source map in directory mode.

This accepts an optional X:\ drive prefix. The prefix requires a backslash, so posix paths, module IDs, Android frames and [native code] parse exactly as before.

Changelog:

 - **[Fix]**: Fix `metro-symbolicate` parsing of stack frames with Windows absolute paths

Test plan:
New test symbolicates the existing fixture with C:\app\-prefixed file names and expects the same output as the original. Without this it fails on every platform:

- thrower.js:18:null
+ throws6@thrower.js:18:null

Also removes symbolicate-test.js from the Windows skip list.

@robhogan
robhogan added this pull request to stack #1940 September 17, 2026 11:47
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 17, 2026
@robhogan
robhogan force-pushed the pr1939 branch 2 times, most recently from a898e90 to 0fa7eff Compare September 29, 2026 17:29
@robhogan
robhogan marked this pull request as ready for review September 29, 2026 17:31
@robhogan
robhogan requested a balanced review from Copilot September 29, 2026 17:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

馃煝 Approval recommended

The focused parser change correctly handles Windows drive prefixes and is covered by regression and existing directory-context tests.

Review effort: Balanced
Findings: None

What changed in this PR

Updates stack-frame parsing to support Windows drive-letter paths while preserving existing formats.

Changes:

  • Accepts X:\ prefixes in stack-frame filenames.
  • Adds cross-platform regression coverage.
  • Re-enables symbolication tests on Windows.
File Description
scripts/鈥媕estFilter.js Removes the obsolete Windows test exclusion.
packages/鈥媘etro-symbolicate/鈥媠rc/鈥婼ymbolication.js Extends stack-frame parsing for Windows paths.
packages/鈥媘etro-symbolicate/鈥媠rc/鈥媉_tests__/鈥媠ymbolicate-test.js Tests Windows-prefixed stack frames.

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Base automatically changed from pr1938 to main September 29, 2026 20:32
Summary:
`metro-symbolicate` parses stack frames with a regex that doesn't allow `:` in file names, so a frame with a Windows absolute path is misparsed at the drive letter:

```
someFunc@D:\app\foo.js:4:0
```

is read as function `D`, file `\app\foo.js`. The match starts at the drive letter, so `someFunc@` is left in the output, and the file name loses its drive - which matters for directory contexts (`symbolicate <dir>`), where the file name is used to find the source map.

This allows an optional `X:\` drive prefix on the function-name-or-file slot and the file slot. Posix paths, module IDs (`123.js`), Android (`bar:4:18063`, `bar:123.js:4:18063`) and `[native code]` frames parse exactly as before - the prefix requires a backslash, so e.g. a single-letter function before a posix path (`a:/js/foo.js:4:1`) is unaffected.

Changelog: [Fix] Fix `metro-symbolicate` stack trace parsing of Windows absolute paths

Test Plan:
Adds a platform-independent test that symbolicates `testfile.stack` with `C:\app\` prefixed file names, and expects the same output as the original. Without this change it fails on all platforms:

```
- thrower.js:18:null
+ throws6@thrower.js:18:null
- thrower.js:30:arguments
+ o@thrower.js:30:arguments
```

Removes `symbolicate-test.js` from the Windows skip list. Its directory context tests use real absolute paths, and previously failed on Windows CI with:

```
- /js/react-native-github/Libraries/BatchedBridge/BatchedBridge.js:23:Object
+ someFunc@/js/react-native-github/Libraries/BatchedBridge/BatchedBridge.js:23:Object
- <testDir>/__fixtures__/directory/fileThatDoesntExist.js:10:null
+ fn@\a\metro\metro\packages\metro-symbolicate\src\__tests__\__fixtures__\directory\fileThatDoesntExist.js:10:null
```

```
yarn jest packages/metro-symbolicate
Tests:       76 passed, 76 total
```
@robhogan
robhogan requested a review from huntie September 29, 2026 20:37
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Sep 29, 2026
@vzaidman
vzaidman self-requested a review September 30, 2026 09:26

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants