flutter / devtools

Performance tools for Flutter
https://flutter.dev/docs/development/tools/devtools/
BSD 3-Clause "New" or "Revised" License
1.55k stars 319 forks source link

Get stacktrace information in release build of DevTools #7392

Open elliette opened 5 months ago

elliette commented 5 months ago

Opening a bug to track work around @sigmundch's suggestions here:

@sigmundch - if we go with option 1, is there a way to specify --no-minify for Flutter web apps? https://github.com/flutter/flutter/issues/62259 indicates the only way is to use the profile build.

@kenzieschmoll for option 2, we would need to update the release process to ship DevTools sourcemap file as well. Wdyt of that approach?

Thanks!

kenzieschmoll commented 5 months ago

If option 2 (shipping the sourcemap file) is the recommended approach for shipping a minified app with meaningful stack traces from production builds, I think this sounds like a good option. @bkonyi @sortie do you see any issues with including a sourcemap file with our build that is uploaded to CIPD and included in the Dart SDK?

sigmundch commented 5 months ago

Re option 1: correct, I believe today the only way is to use --profile unless we add additional flags to the flutter tool or allow specifying arbitrary dart2js flags (request in https://github.com/flutter/flutter/issues/99015). Good news is that today --profile seems to only pass --no-minify on top of what --release has, so it happens to match the flags we want.

Re option 2: note that the deobfuscation process will be done offline, so access to the sourcemap is not needed while the app is running. As such, the sourcemap file doesn't need to be shipped with the app, it just needs to be built and archived in a way that can be retrieved easily for each individual released version of the app. Then, when a report comes in (either through user reporting or crash analytics, if supported), you can retrieve the .map file that corresponds to the app that produced the crash.

Today we have a .zip created in this repo for each release tag, if that zip included the compiled .js and .map file, that would do. It would also work if we upload the .map file to CIPD, but we can exclude it when we create the Dart SDK binary.

sortie commented 5 months ago

@kenzieschmoll You can include any files you wish in the devtools_shared and devtools_app/build/web directories per the recipe https://dart.googlesource.com/recipes/+/refs/heads/main/recipes/roller/devtools.py#78

I have no opinion on the technical aspects of a source map, don't know the tech, I just package it :)

bkonyi commented 5 months ago

If option 2 (shipping the sourcemap file) is the recommended approach for shipping a minified app with meaningful stack traces from production builds, I think this sounds like a good option. @bkonyi @sortie do you see any issues with including a sourcemap file with our build that is uploaded to CIPD and included in the Dart SDK?

I don't think it'd be a problem, but we should always keep SDK size in mind when adding new artifacts. How big is the source map?

sigmundch commented 5 months ago

... we should always keep SDK size in mind when adding new artifacts. How big is the source map?

FWIW, this is why I was highlighting above that shipping it with the SDK is optional. It is enough to build it and save it in CIPD so we can retrieve it later for deobfsucation, we just need to have a reliable way to find the .map file corresponding to a release.

bkonyi commented 5 months ago

... we should always keep SDK size in mind when adding new artifacts. How big is the source map?

FWIW, this is why I was highlighting above that shipping it with the SDK is optional. It is enough to build it and save it in CIPD so we can retrieve it later for deobfsucation, we just need to have a reliable way to find the .map file corresponding to a release.

Oops, I clearly didn't read the entire thread... :) that makes sense. If we package it with the CIPD release of DevTools, it shouldn't be too hard to find it if we know the Dart SDK version.

sigmundch commented 5 months ago

Oops, I clearly didn't read the entire thread... :)

haha. I also didn't give a clear rationale on my earlier post, so I was just connecting the two thoughts :smile: