Skip to content

Commit 054b814

Browse files
committed
More safety.
1 parent 67e2fe2 commit 054b814

File tree

2 files changed

+86
-4
lines changed

2 files changed

+86
-4
lines changed

packages/common_client/lib/src/plugins/operations.dart

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,17 @@ import 'package:launchdarkly_dart_common/launchdarkly_dart_common.dart'
44
import 'plugin.dart';
55
import '../hooks/hook.dart';
66

7+
const _unknownPlugin = 'unknown';
8+
9+
String safeGetPluginName<TClient>(PluginBase<TClient> plugin, LDLogger logger) {
10+
try {
11+
return plugin.metadata.name;
12+
} catch (err) {
13+
logger.warn('Exception thrown access the name of a registered plugin.');
14+
return _unknownPlugin;
15+
}
16+
}
17+
718
List<Hook>? safeGetHooks<TClient>(
819
List<PluginBase<TClient>>? plugins, LDLogger logger) {
920
if (plugins == null) return null;
@@ -14,7 +25,7 @@ List<Hook>? safeGetHooks<TClient>(
1425
return plugin.hooks;
1526
} catch (err) {
1627
logger.warn(
17-
'Exception thrown getting hooks for plugin ${plugin.metadata.name}. Unable to get hooks for plugin.');
28+
'Exception thrown getting hooks for plugin ${safeGetPluginName(plugin, logger)}. Unable to get hooks for plugin.');
1829
}
1930
return [];
2031
})
@@ -32,7 +43,7 @@ void safeRegisterPlugins<TClient>(
3243
plugin.register(client, metadata);
3344
} catch (err) {
3445
logger.warn(
35-
'Exception thrown when registering plugin ${plugin.metadata.name}');
46+
'Exception thrown when registering plugin ${safeGetPluginName(plugin, logger)}');
3647
}
3748
});
3849
}

packages/common_client/test/plugins/operations_test.dart

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,26 @@ final class TestPlugin extends PluginBase<dynamic> {
2424
final List<Hook> _hooks;
2525
final bool shouldThrowOnGetHooks;
2626
final bool shouldThrowOnRegister;
27+
final bool shouldThrowOnGetMetadata;
2728
final PluginMetadata _metadata;
2829
int registerCallCount = 0;
2930
dynamic lastClientReceived;
3031
PluginEnvironmentMetadata? lastEnvironmentMetadataReceived;
3132

3233
TestPlugin(this.pluginName, this._hooks,
33-
{this.shouldThrowOnGetHooks = false, this.shouldThrowOnRegister = false})
34+
{this.shouldThrowOnGetHooks = false,
35+
this.shouldThrowOnRegister = false,
36+
this.shouldThrowOnGetMetadata = false})
3437
: _metadata = PluginMetadata(name: pluginName);
3538

3639
@override
37-
PluginMetadata get metadata => _metadata;
40+
PluginMetadata get metadata {
41+
if (shouldThrowOnGetMetadata) {
42+
throw Exception(
43+
'Test exception accessing metadata for plugin $pluginName');
44+
}
45+
return _metadata;
46+
}
3847

3948
@override
4049
List<Hook> get hooks {
@@ -248,6 +257,36 @@ void main() {
248257
record.level == LDLogLevel.warn &&
249258
record.message.contains('plugin-3'))))).called(1);
250259
});
260+
261+
test('handles plugin that throws when accessing metadata', () {
262+
final hook1 = TestHook('hook-1');
263+
final plugin1 = TestPlugin('plugin-1', [hook1]);
264+
final plugin2 = TestPlugin('plugin-2', [],
265+
shouldThrowOnGetHooks: true, shouldThrowOnGetMetadata: true);
266+
final hook3 = TestHook('hook-3');
267+
final plugin3 = TestPlugin('plugin-3', [hook3]);
268+
269+
final result = safeGetHooks([plugin1, plugin2, plugin3], logger);
270+
271+
expect(result, isNotNull);
272+
expect(result!.length, equals(2));
273+
expect(result, containsAll([hook1, hook3]));
274+
275+
// Verify warning was logged mentioning unknown plugin since metadata failed
276+
// This happens when safeGetHooks tries to get the plugin name for logging after hooks fails
277+
verify(() => mockLogAdapter.log(any(
278+
that: predicate<LDLogRecord>((record) =>
279+
record.level == LDLogLevel.warn &&
280+
record.message.contains('unknown'))))).called(1);
281+
282+
// Also verify that the metadata access failure was logged
283+
verify(() => mockLogAdapter.log(any(
284+
that: predicate<LDLogRecord>((record) =>
285+
record.level == LDLogLevel.warn &&
286+
record.message.contains(
287+
'Exception thrown access the name of a registered plugin')))))
288+
.called(1);
289+
});
251290
});
252291

253292
group('safeRegisterPlugins', () {
@@ -452,5 +491,37 @@ void main() {
452491
expect(plugin2.lastEnvironmentMetadataReceived,
453492
same(customEnvironmentMetadata));
454493
});
494+
495+
test(
496+
'handles plugin that throws when accessing metadata during registration',
497+
() {
498+
final plugin1 = TestPlugin('plugin-1', []);
499+
final plugin2 = TestPlugin('plugin-2', [],
500+
shouldThrowOnRegister: true, shouldThrowOnGetMetadata: true);
501+
final plugin3 = TestPlugin('plugin-3', []);
502+
503+
safeRegisterPlugins(testClient, testEnvironmentMetadata,
504+
[plugin1, plugin2, plugin3], logger);
505+
506+
// All plugins should have attempted registration
507+
expect(plugin1.registerCallCount, equals(1));
508+
expect(plugin2.registerCallCount, equals(1));
509+
expect(plugin3.registerCallCount, equals(1));
510+
511+
// Verify warning was logged mentioning unknown plugin since metadata failed
512+
// This happens when safeRegisterPlugins tries to log the plugin name after registration fails
513+
verify(() => mockLogAdapter.log(any(
514+
that: predicate<LDLogRecord>((record) =>
515+
record.level == LDLogLevel.warn &&
516+
record.message.contains('unknown'))))).called(1);
517+
518+
// Also verify that the metadata access failure was logged
519+
verify(() => mockLogAdapter.log(any(
520+
that: predicate<LDLogRecord>((record) =>
521+
record.level == LDLogLevel.warn &&
522+
record.message.contains(
523+
'Exception thrown access the name of a registered plugin')))))
524+
.called(1);
525+
});
455526
});
456527
}

0 commit comments

Comments
 (0)