Skip to content

Commit c1784cd

Browse files
authored
Fix: Use App Name if Package Name is not available (#411)
* Fix: Use App Name if Package Name is not available * add comment describing the code * replace invalid chars with underscore * Formatting * Improve comment * Add PR ID to changelog * Use TestFixture * improve test fixture
1 parent 4d42b29 commit c1784cd

File tree

3 files changed

+81
-22
lines changed

3 files changed

+81
-22
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

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

67
# 5.0.0
78

flutter/lib/src/default_integrations.dart

+20-1
Original file line numberDiff line numberDiff line change
@@ -367,8 +367,15 @@ class LoadReleaseIntegration extends Integration<SentryFlutterOptions> {
367367
if (!kIsWeb) {
368368
if (options.release == null || options.dist == null) {
369369
final packageInfo = await _packageLoader();
370+
var name = packageInfo.packageName;
371+
if (name.isEmpty) {
372+
// Not all platforms have a packageName.
373+
// If no packageName is available, use the appName instead.
374+
name = _cleanAppName(packageInfo.appName);
375+
}
376+
370377
final release =
371-
'${packageInfo.packageName}@${packageInfo.version}+${packageInfo.buildNumber}';
378+
'$name@${packageInfo.version}+${packageInfo.buildNumber}';
372379
options.logger(SentryLevel.debug, 'release: $release');
373380

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

383390
options.sdk.addIntegration('loadReleaseIntegration');
384391
}
392+
393+
String _cleanAppName(String appName) {
394+
// Replace disallowed chars with an underscore '_'
395+
// https://docs.sentry.io/platforms/flutter/configuration/releases/#bind-the-version
396+
return appName
397+
.replaceAll('/', '_')
398+
.replaceAll('\\', '_')
399+
.replaceAll('\t', '_')
400+
.replaceAll('\r\n', '_')
401+
.replaceAll('\r', '_')
402+
.replaceAll('\n', '_');
403+
}
385404
}

flutter/test/default_integrations_test.dart

+60-21
Original file line numberDiff line numberDiff line change
@@ -227,40 +227,79 @@ void main() {
227227
});
228228

229229
group('$LoadReleaseIntegration', () {
230-
Future<PackageInfo> loadRelease() {
231-
return Future.value(PackageInfo(
232-
appName: 'sentry_flutter',
233-
packageName: 'foo.bar',
234-
version: '1.2.3',
235-
buildNumber: '789',
236-
));
237-
}
230+
late Fixture fixture;
231+
232+
setUp(() {
233+
fixture = Fixture();
234+
});
238235

239236
test('does not overwrite options', () async {
240-
final options = SentryFlutterOptions(dsn: fakeDsn);
241-
options.release = '1.0.0';
242-
options.dist = 'dist';
237+
fixture.options.release = '1.0.0';
238+
fixture.options.dist = 'dist';
243239

244-
final integration = LoadReleaseIntegration(loadRelease);
245-
await integration.call(MockHub(), options);
240+
await fixture.getIntegration().call(MockHub(), fixture.options);
246241

247-
expect(options.release, '1.0.0');
248-
expect(options.dist, 'dist');
242+
expect(fixture.options.release, '1.0.0');
243+
expect(fixture.options.dist, 'dist');
249244
});
250245

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

254-
final integration = LoadReleaseIntegration(loadRelease);
255-
await integration.call(MockHub(), options);
249+
expect(fixture.options.release, '[email protected]+789');
250+
expect(fixture.options.dist, '789');
251+
});
252+
253+
test('sets app name as in release if packagename is empty', () async {
254+
final loader = () {
255+
return Future.value(PackageInfo(
256+
appName: 'sentry_flutter',
257+
packageName: '',
258+
version: '1.2.3',
259+
buildNumber: '789',
260+
));
261+
};
262+
await fixture
263+
.getIntegration(loader: loader)
264+
.call(MockHub(), fixture.options);
265+
266+
expect(fixture.options.release, '[email protected]+789');
267+
expect(fixture.options.dist, '789');
268+
});
256269

257-
expect(options.release, '[email protected]+789');
258-
expect(options.dist, '789');
270+
test('release name does not contain ivalid chars', () async {
271+
final loader = () {
272+
return Future.value(PackageInfo(
273+
appName: '\\/sentry\tflutter \r\nfoo\nbar\r',
274+
packageName: '',
275+
version: '1.2.3',
276+
buildNumber: '789',
277+
));
278+
};
279+
await fixture
280+
.getIntegration(loader: loader)
281+
.call(MockHub(), fixture.options);
282+
283+
expect(fixture.options.release, '__sentry_flutter [email protected]+789');
284+
expect(fixture.options.dist, '789');
259285
});
260286
});
261287
}
262288

263289
class Fixture {
264290
final hub = MockHub();
265-
final options = SentryFlutterOptions();
291+
final options = SentryFlutterOptions(dsn: fakeDsn);
292+
293+
LoadReleaseIntegration getIntegration({PackageLoader? loader}) {
294+
return LoadReleaseIntegration(loader ?? loadRelease);
295+
}
296+
297+
Future<PackageInfo> loadRelease() {
298+
return Future.value(PackageInfo(
299+
appName: 'sentry_flutter',
300+
packageName: 'foo.bar',
301+
version: '1.2.3',
302+
buildNumber: '789',
303+
));
304+
}
266305
}

0 commit comments

Comments
 (0)