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.

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

Implement hermes source maps #21

Closed
mitsuhiko opened this issue Nov 6, 2019 · 2 comments · Fixed by #22
Closed

Implement hermes source maps #21

mitsuhiko opened this issue Nov 6, 2019 · 2 comments · Fixed by #22

Comments

@mitsuhiko
Copy link
Member

mitsuhiko commented Nov 6, 2019

We currently cannot handle hermes source maps but it should be possible with some larger changes in the library (refs getsentry/sentry-react-native#649)

We probably want to normalize x_facebook_sources somehow because that contains scope mapping information which is exactly what we need. This means we no longer need to access minified sources for figuring out the function name.

Rough Overview

Hermes internally compiles javascript sources to bytecode directly which means there is actually no minified javascript behind the scenes. Currently we use the minified source to figure out a few things that are crucial for processing:

  • we use the comment in the minified file to locate the source map (
    pub fn locate_sourcemap_reference<R: Read>(rdr: R) -> Result<SourceMapRef> {
    for line in BufReader::new(rdr).lines() {
    let line = line?;
    if line.starts_with("//# sourceMappingURL=") || line.starts_with("//@ sourceMappingURL=") {
    let url = str::from_utf8(&line.as_bytes()[21..])?.trim().to_owned();
    if line.starts_with("//@") {
    return Ok(SourceMapRef::LegacyRef(url));
    } else {
    return Ok(SourceMapRef::Ref(url));
    }
    }
    }
    Ok(SourceMapRef::Missing)
    }
    )
  • we use the minified file to reverse tokenize from the error location to the function scope (heuristic) (

    rust-sourcemap/src/types.rs

    Lines 592 to 601 in a086b92

    pub fn get_original_function_name<'a>(
    &self,
    line: u32,
    col: u32,
    minified_name: &str,
    sv: &'a SourceView<'a>,
    ) -> Option<&str> {
    self.lookup_token(line, col)
    .and_then(|token| sv.get_original_function_name(token, minified_name))
    }
    )

These two things are generally used by sentry and assume some behavior already. In particular we assume that the reported function name is the minified file.

With Hermes both of those assumptions are invalidated. First of all because there is no minified file we cannot use our heuristics. Thankfully though Hermes source maps have a second mapping in them called x_facebook_sources which contains mappings from a token location to the enclosing function. The metro symbolicator already uses this to map back to a function name.

Secondly because there is just one huge source map for all files in a react native app we need a separate algorithm to map from the reported filename in a stack to the section in the source map. In the past (with RAM bundles) we rewrote them to virtual files. (see the ram bundle code). Here we probably want to add an abstraction at a higher level where we provide a file name and it returns where in the source map it would be and which source index in it (as there are multiple virtual files in it).

This means we want to change the "get original function name" logic to make the minified source view optional for the hermes case where the source map has enough information to get back to the original name.

Changes in Sentry

After we fixed this here we also need to do some changes in sentry obviously. Currently we rewrite source maps in sentry-cli on the way up and if they have not been rewritten then, they are rewritten again by sentry on loading. We probably do not want to do anything like this with hermes source maps.

The changes there will likely be these:

  1. use the 0 based indexing for hermes instead of 1 indexed like we do for normal source maps
  2. add a layer of indirection for selecting source maps for hermes / react-native to not look for the minified source file but navigate to the source map directly (assume it always comes with the same fixed name?)
  3. not pass the minified file when looking for the original function name.
@teampolyglot
Copy link

I think adding a couple of failing tests in a new branch would be a good starting point.

@kc-curefit
Copy link

Any timelines on when this would be available ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants