Skip to content

Fix: Use App Name if Package Name is not available #411

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

Merged
merged 8 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Fix: `Sentry.close()` closes native SDK integrations (#388)
* Fix: Mark `Sentry.currentHub` as deprecated (#406)
* Fix: Use name from pubspec.yaml for release if package id is not available (#411)

# 5.0.0

Expand Down
21 changes: 20 additions & 1 deletion flutter/lib/src/default_integrations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,15 @@ class LoadReleaseIntegration extends Integration<SentryFlutterOptions> {
if (!kIsWeb) {
if (options.release == null || options.dist == null) {
final packageInfo = await _packageLoader();
var name = packageInfo.packageName;
if (name.isEmpty) {
// Not all platforms have a packageName.
// If no packageName is available, use the appName instead.
name = _cleanAppName(packageInfo.appName);
}

final release =
'${packageInfo.packageName}@${packageInfo.version}+${packageInfo.buildNumber}';
'$name@${packageInfo.version}+${packageInfo.buildNumber}';
options.logger(SentryLevel.debug, 'release: $release');

options.release = options.release ?? release;
Expand All @@ -382,4 +389,16 @@ class LoadReleaseIntegration extends Integration<SentryFlutterOptions> {

options.sdk.addIntegration('loadReleaseIntegration');
}

String _cleanAppName(String appName) {
// Replace disallowed chars with an underscore '_'
// https://docs.sentry.io/platforms/flutter/configuration/releases/#bind-the-version
return appName
.replaceAll('/', '_')
.replaceAll('\\', '_')
.replaceAll('\t', '_')
.replaceAll('\r\n', '_')
.replaceAll('\r', '_')
.replaceAll('\n', '_');
}
}
84 changes: 64 additions & 20 deletions flutter/test/default_integrations_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -227,35 +227,61 @@ void main() {
});

group('$LoadReleaseIntegration', () {
Future<PackageInfo> loadRelease() {
return Future.value(PackageInfo(
appName: 'sentry_flutter',
packageName: 'foo.bar',
version: '1.2.3',
buildNumber: '789',
));
}
late LoadReleaseIntegrationFixture fixture;

setUp(() {
fixture = LoadReleaseIntegrationFixture();
});

test('does not overwrite options', () async {
final options = SentryFlutterOptions(dsn: fakeDsn);
options.release = '1.0.0';
options.dist = 'dist';
fixture.options.release = '1.0.0';
fixture.options.dist = 'dist';

final integration = LoadReleaseIntegration(loadRelease);
await integration.call(MockHub(), options);
await fixture.getIntegration().call(MockHub(), fixture.options);

expect(options.release, '1.0.0');
expect(options.dist, 'dist');
expect(fixture.options.release, '1.0.0');
expect(fixture.options.dist, 'dist');
});

test('sets release and dist if not set on options', () async {
final options = SentryFlutterOptions(dsn: fakeDsn);
await fixture.getIntegration().call(MockHub(), fixture.options);

expect(fixture.options.release, '[email protected]+789');
expect(fixture.options.dist, '789');
});

final integration = LoadReleaseIntegration(loadRelease);
await integration.call(MockHub(), options);
test('sets app name as in release if packagename is empty', () async {
final loader = () {
return Future.value(PackageInfo(
appName: 'sentry_flutter',
packageName: '',
version: '1.2.3',
buildNumber: '789',
));
};
await fixture
.getIntegration(loader: loader)
.call(MockHub(), fixture.options);

expect(fixture.options.release, '[email protected]+789');
expect(fixture.options.dist, '789');
});

expect(options.release, '[email protected]+789');
expect(options.dist, '789');
test('release name does not contain ivalid chars', () async {
final loader = () {
return Future.value(PackageInfo(
appName: '\\/sentry\tflutter \r\nfoo\nbar\r',
packageName: '',
version: '1.2.3',
buildNumber: '789',
));
};
await fixture
.getIntegration(loader: loader)
.call(MockHub(), fixture.options);

expect(fixture.options.release, '__sentry_flutter [email protected]+789');
expect(fixture.options.dist, '789');
});
});
}
Expand All @@ -264,3 +290,21 @@ class Fixture {
final hub = MockHub();
final options = SentryFlutterOptions();
}

class LoadReleaseIntegrationFixture {
final hub = MockHub();
final options = SentryFlutterOptions(dsn: fakeDsn);

LoadReleaseIntegration getIntegration({PackageLoader? loader}) {
return LoadReleaseIntegration(loader ?? loadRelease);
}

Future<PackageInfo> loadRelease() {
return Future.value(PackageInfo(
appName: 'sentry_flutter',
packageName: 'foo.bar',
version: '1.2.3',
buildNumber: '789',
));
}
}