Skip to content

Commit

Permalink
fix: upload only known build output directories (#277)
Browse files Browse the repository at this point in the history
* change build dir default value

* add symbols path config & enumarate known build dir paths

* collect symbol files from all dirs

* add macos & ios rules

* chore: add changelog

* add missing targets

* trim printed logs

* upload build files on failure

* update apk test

* add windows pdb to upload

* comments
  • Loading branch information
vaind authored Nov 15, 2024
1 parent 474376f commit 22b2b33
Show file tree
Hide file tree
Showing 12 changed files with 214 additions and 76 deletions.
18 changes: 16 additions & 2 deletions .github/workflows/integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,30 @@ on:

jobs:
integration-test:
name: ${{ matrix.target }} @ ${{ matrix.host }}
name: ${{ matrix.target }}
runs-on: ${{ matrix.host }}-latest
strategy:
fail-fast: false
matrix:
include:
- host: macos
target: macos
- host: macos
target: macos-framework
- host: macos
target: ios
- host: macos
target: ios-framework
- host: macos
target: ipa
- host: ubuntu
target: linux
- host: ubuntu
target: web
- host: ubuntu
target: android
target: apk
- host: ubuntu
target: appbundle
- host: windows
target: windows
env:
Expand All @@ -51,3 +59,9 @@ jobs:
key: integration-test-${{ matrix.host }}-${{ matrix.target }}-${{ hashFiles('flutter.version') }}

- run: dart test --tags integration

- uses: actions/upload-artifact@v4
if: failure()
with:
name: ${{ matrix.target }}-build
path: temp/testapp-${{ matrix.target }}/build
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

### Changes

- Upload debug symbols for known release build paths and the configured `symbols_path` ([#277](https://github.com/getsentry/sentry-dart-plugin/pull/277))
Previously, all debug symbols recognized by Sentry CLI were uploaded (starting in the current directory by default).
Now, the plugin checks the paths where `flutter build` outputs debug symbols for release builds and only uploads those.

### Features

- Add urlPrefix to sentry configuration ([#253](https://github.com/getsentry/sentry-dart-plugin/pull/253))
Expand Down Expand Up @@ -58,7 +64,7 @@

### Fixes

- Updated the `process` dependency range to `>=4.2.4 <6.0.0` ([#202](https://github.com/getsentry/sentry-dart-plugin/pull/202)).
- Updated the `process` dependency range to `>=4.2.4 <6.0.0` ([#202](https://github.com/getsentry/sentry-dart-plugin/pull/202)).
- This update resolves a version conflict issue when using the `integration_test` package with Flutter version `3.19.0`

## 1.7.0
Expand All @@ -70,7 +76,7 @@
### Dependencies

- Bump CLI from v2.21.2 to v2.27.0 ([#180](https://github.com/getsentry/sentry-dart-plugin/pull/180), [#195](https://github.com/getsentry/sentry-dart-plugin/pull/195), [#196](https://github.com/getsentry/sentry-dart-plugin/pull/196))
- [changelog](https://github.com/getsentry/sentry-cli/blob/master/CHANGELOG.md#2270)
- [changelog](https://github.com/getsentry/sentry-cli/blob/master/CHANGELOG.md#2270)
- [diff](https://github.com/getsentry/sentry-cli/compare/2.21.2...2.27.0)

## 1.6.3
Expand Down
9 changes: 7 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ sentry:
log_level: error # possible values: trace, debug, info, warn, error
release: ...
dist: ...
build_path: ...
web_build_path: ...
symbols_path: ...
commits: auto
ignore_missing: true
```
Expand Down Expand Up @@ -80,7 +82,9 @@ wait_for_processing=false
log_level=error # possible values: trace, debug, info, warn, error
release=...
dist=...
build_path: ...
web_build_path=...
symbols_path=...
commits=auto
ignore_missing=true
```
Expand All @@ -101,8 +105,9 @@ ignore_missing=true
| log_level | Configures the log level for sentry-cli | warn (string) | no | SENTRY_LOG_LEVEL |
| release | The release version for source maps, it should match the release set by the SDK | name@version from pubspec (string) | no | SENTRY_RELEASE |
| dist | The dist/build number for source maps, it should match the dist set by the SDK | the number after the '+' char from 'version' pubspec (string) | no | SENTRY_DIST |
| build_path | The build folder of debug files for upload | `.` current folder (string) | no | - |
| web_build_path | The web build folder of debug files for upload | `build/web` relative to build_path (string) | no | - |
| build_path | The build folder of debug files for upload | `build` (string) | no | - |
| web_build_path | The web build folder of debug files for upload relative to build_path | `web` (string) | no | - |
| symbols_path | The directory containing debug symbols (i.e. the `--split-debug-info=` parameter value you pass to `flutter build`) | `.` (string) | no | - |
| commits | Release commits integration | auto (string) | no | - |
| ignore_missing | Ignore missing commits previously used in the release | false (boolean) | no | - |
| bin_dir | The folder where the plugin downloads the sentry-cli binary | .dart_tool/pub/bin/sentry_dart_plugin (string) | no | - |
Expand Down
8 changes: 7 additions & 1 deletion example/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,15 @@ sentry:
# default 'warning'
log_level: error

# default to build/web
# default 'build'
#build_path: ...

# default 'web' (relative to build_path, i.e. resolves to 'build/web')
#web_build_path: ...

# default '.'
#symbols_path: ...

# default to name@version from pubspec
#release: ...
```
80 changes: 77 additions & 3 deletions lib/sentry_dart_plugin.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import 'dart:convert';

import 'package:file/file.dart';
import 'package:process/process.dart';
import 'package:sentry_dart_plugin/src/utils/extensions.dart';

import 'src/configuration.dart';
import 'src/utils/injector.dart';
Expand All @@ -10,6 +12,7 @@ import 'src/utils/log.dart';
/// debug symbols and source maps
class SentryDartPlugin {
late Configuration _configuration;
final symbolFileRegexp = RegExp(r'[/\\]app[^/\\]+.*\.(dSYM|symbols)$');

/// SentryDartPlugin ctor. that inits the injectors
SentryDartPlugin() {
Expand Down Expand Up @@ -75,15 +78,86 @@ class SentryDartPlugin {
Log.info('includeSources is disabled, not uploading sources.');
}

params.add(_configuration.buildFilesFolder);

_addWait(params);

await _executeAndLog('Failed to upload symbols', params);
final debugSymbolPaths = _enumerateDebugSymbolPaths();
final fs = injector.get<FileSystem>();
await for (final path in debugSymbolPaths) {
if (await fs.directory(path).exists() || await fs.file(path).exists()) {
await _executeAndLog('Failed to upload symbols', [...params, path]);
}
}

for (final path in await _enumerateSymbolFiles()) {
await _executeAndLog('Failed to upload symbols', [...params, path]);
}

Log.taskCompleted(taskName);
}

Stream<String> _enumerateDebugSymbolPaths() async* {
final buildDir = _configuration.buildFilesFolder;

// Android (apk, appbundle)
yield '$buildDir/app/outputs';
yield '$buildDir/app/intermediates';

// Windows
for (final subdir in ['', '/x64', '/arm64']) {
yield '$buildDir/windows$subdir/runner/Release';
}
// TODO we should delete this once we have windows symbols collected automatically.
// Related to https://github.com/getsentry/sentry-dart-plugin/issues/173
yield 'windows/flutter/ephemeral/flutter_windows.dll.pdb';

// Linux
for (final subdir in ['/x64', '/arm64']) {
yield '$buildDir/linux$subdir/release/bundle';
}

// macOS
yield '$buildDir/macos/Build/Products/Release';

// macOS (macOS-framework)
yield '$buildDir/macos/framework/Release';

// iOS
yield '$buildDir/ios/iphoneos/Runner.App';
yield '$buildDir/ios/Release-iphoneos';

// iOS (ipa)
yield '$buildDir/ios/archive';

// iOS (ios-framework)
yield '$buildDir/ios/framework/Release';
}

Future<Set<String>> _enumerateSymbolFiles() async {
final result = <String>{};
final fs = injector.get<FileSystem>();

if (_configuration.symbolsFolder.isNotEmpty) {
final symbolsRootDir = fs.directory(_configuration.symbolsFolder);
if (await symbolsRootDir.exists()) {
await for (final entry in symbolsRootDir.find(symbolFileRegexp)) {
result.add(entry.path);
}
}
}

// for backward compatibility, also check the build dir if it has been
// configured with a different path.
if (_configuration.buildFilesFolder != _configuration.symbolsFolder) {
final symbolsRootDir = fs.directory(_configuration.buildFilesFolder);
if (await symbolsRootDir.exists()) {
await for (final entry in symbolsRootDir.find(symbolFileRegexp)) {
result.add(entry.path);
}
}
}
return result;
}

List<String> _releasesCliParams() {
final params = <String>[];
_setUrlAndTokenAndLog(params);
Expand Down
19 changes: 11 additions & 8 deletions lib/src/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ import 'utils/log.dart';

class Configuration {
late final FileSystem _fs = injector.get<FileSystem>();
// cannot use ${Directory.current.path}/build since --split-debug-info allows
// setting a custom path which is a sibling of build
/// The Build folder, defaults to the current directory.

/// The Build folder, defaults `build`.
late String buildFilesFolder;

/// Whether to upload debug symbols, defaults to true
Expand Down Expand Up @@ -67,9 +66,13 @@ class Configuration {
/// The Apps name, defaults to [name] from pubspec
late String name;

/// the Web Build folder, defaults to build/web
/// the Web Build folder, defaults to `web`.
/// Relative to the [buildFilesFolder] so the default resolves to `build/web`.
late String webBuildFilesFolder;

/// The directory passed to `--split-debug-info`, defaults to '.'
late String symbolsFolder;

/// The URL prefix, defaults to null
late String? urlPrefix;

Expand Down Expand Up @@ -141,13 +144,13 @@ class Configuration {
commits = configValues.commits ?? 'auto';
ignoreMissing = configValues.ignoreMissing ?? false;

buildFilesFolder = configValues.buildPath ?? _fs.currentDirectory.path;
buildFilesFolder = configValues.buildPath ?? 'build';
// uploading JS and Map files need to have the correct folder structure
// otherwise symbolication fails, the default path for the web build folder is build/web
// otherwise symbolication fails, the default path for the web build folder is web
// but can be customized so making it flexible.
final webBuildPath =
configValues.webBuildPath ?? _fs.path.join('build', 'web');
final webBuildPath = configValues.webBuildPath ?? 'web';
webBuildFilesFolder = _fs.path.join(buildFilesFolder, webBuildPath);
symbolsFolder = configValues.symbolsPath ?? '.';

project = configValues.project; // or env. var. SENTRY_PROJECT
org = configValues.org; // or env. var. SENTRY_ORG
Expand Down
5 changes: 5 additions & 0 deletions lib/src/configuration_values.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class ConfigurationValues {
final String? dist;
final String? buildPath;
final String? webBuildPath;
final String? symbolsPath;
final String? commits;
final bool? ignoreMissing;
final String? binDir;
Expand All @@ -43,6 +44,7 @@ class ConfigurationValues {
this.dist,
this.buildPath,
this.webBuildPath,
this.symbolsPath,
this.commits,
this.ignoreMissing,
this.binDir,
Expand Down Expand Up @@ -94,6 +96,7 @@ class ConfigurationValues {
dist: sentryArguments['dist'],
buildPath: sentryArguments['build_path'],
webBuildPath: sentryArguments['web_build_path'],
symbolsPath: sentryArguments['symbols_path'],
commits: sentryArguments['commits'],
ignoreMissing: boolFromString(sentryArguments['ignore_missing']),
binDir: sentryArguments['bin_dir'],
Expand Down Expand Up @@ -127,6 +130,7 @@ class ConfigurationValues {
dist: configReader.getString('dist'),
buildPath: configReader.getString('build_path'),
webBuildPath: configReader.getString('web_build_path'),
symbolsPath: configReader.getString('symbols_path'),
commits: configReader.getString('commits'),
ignoreMissing: configReader.getBool('ignore_missing'),
binDir: configReader.getString('bin_dir'),
Expand Down Expand Up @@ -180,6 +184,7 @@ class ConfigurationValues {
dist: platformEnv.dist ?? args.dist ?? file.dist,
buildPath: args.buildPath ?? file.buildPath,
webBuildPath: args.webBuildPath ?? file.webBuildPath,
symbolsPath: args.symbolsPath ?? file.symbolsPath,
commits: args.commits ?? file.commits,
ignoreMissing: args.ignoreMissing ?? file.ignoreMissing,
binDir: args.binDir ?? file.binDir,
Expand Down
12 changes: 12 additions & 0 deletions lib/src/utils/extensions.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,16 @@
import 'package:file/file.dart';

/// Checks if the given String == null
extension StringValidations on String? {
bool get isNull => this == null;
}

extension DirectorySearch on Directory {
Stream<FileSystemEntity> find(RegExp regexp) async* {
await for (final entity in list(recursive: true)) {
if (regexp.hasMatch(entity.path)) {
yield entity;
}
}
}
}
12 changes: 7 additions & 5 deletions test/configuration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ void main() {
dist: 'dist-args-config',
buildPath: 'build_path-args-config',
webBuildPath: 'web_build_path-args-config',
symbolsPath: 'symbols_path-args-config',
commits: 'commits-args-config',
ignoreMissing: true,
binDir: 'binDir-args-config',
Expand All @@ -93,6 +94,7 @@ void main() {
dist: 'dist-file-config',
buildPath: 'build_path-file-config',
webBuildPath: 'web_build_path-file-config',
symbolsPath: 'symbols_path-args-config',
commits: 'commits-file-config',
ignoreMissing: false,
binDir: 'binDir-file-config',
Expand Down Expand Up @@ -122,6 +124,7 @@ void main() {
expect(sut.release, 'release-args-config');
expect(sut.dist, 'dist-args-config');
expect(sut.buildFilesFolder, 'build_path-args-config');
expect(sut.symbolsFolder, 'symbols_path-args-config');
expect(
sut.webBuildFilesFolder,
fixture.fs.path.join(
Expand Down Expand Up @@ -157,6 +160,7 @@ void main() {
dist: 'dist-file-config',
buildPath: 'build_path-file-config',
webBuildPath: 'web_build_path-file-config',
symbolsPath: 'symbols_path-args-config',
commits: 'commits-file-config',
ignoreMissing: true,
binDir: 'binDir-file-config',
Expand Down Expand Up @@ -187,6 +191,7 @@ void main() {
expect(sut.release, 'release-file-config');
expect(sut.dist, 'dist-file-config');
expect(sut.buildFilesFolder, 'build_path-file-config');
expect(sut.symbolsFolder, 'symbols_path-args-config');
expect(
sut.webBuildFilesFolder,
fixture.fs.path
Expand Down Expand Up @@ -218,13 +223,10 @@ void main() {
expect(sut.uploadSources, false);
expect(sut.commits, 'auto');
expect(sut.ignoreMissing, false);
expect(
sut.buildFilesFolder,
fixture.fs.currentDirectory.path,
);
expect(sut.buildFilesFolder, 'build');
expect(
sut.webBuildFilesFolder,
fixture.fs.path.join(sut.buildFilesFolder, 'build/web'),
fixture.fs.path.join(sut.buildFilesFolder, 'web'),
);
expect(sut.waitForProcessing, false);
expect(sut.binDir, '.dart_tool/pub/bin/sentry_dart_plugin');
Expand Down
Loading

0 comments on commit 22b2b33

Please sign in to comment.