diff --git a/CHANGELOG.md b/CHANGELOG.md index ece0dc2..40746eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -342,6 +342,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `copyWithBaggage` work without an installed SDK (previously `StateError`), per "The Baggage API MUST be fully functional in the absence of an installed SDK." +- **`TraceState.put` / `remove` never throw.** Per the trace API spec, + mutating operations validate input and "MUST NOT return `TraceState` + containing invalid data" while following the error-handling guidelines — + invalid keys/values are now ignored with a warning (previously + `ArgumentError`), and both operations work without an installed SDK + (previously `StateError`). ## [1.0.0-beta.9] - 2026-07-11 diff --git a/lib/src/api/trace/trace_state.dart b/lib/src/api/trace/trace_state.dart index a85bc0a..7ff2119 100644 --- a/lib/src/api/trace/trace_state.dart +++ b/lib/src/api/trace/trace_state.dart @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import '../../factory/otel_factory.dart'; +import '../../util/otel_log.dart'; part 'trace_state_create.dart'; @@ -74,19 +75,18 @@ class TraceState { /// If adding this pair would exceed the 32 key-value pair limit, /// the oldest entries are removed to make room. TraceState put(String key, String value) { - if (OTelFactory.otelFactory == null) { - throw StateError('Call initialize() first.'); - } if (!_isValidKey(key) || !_isValidValue(value)) { - throw ArgumentError('Invalid key or value for TraceState'); + OTelLog.warn('Invalid TraceState key or value; entry ignored.'); + return this; } + final factory = OTelFactory.getOrCreateDefault(); final newEntries = Map.from(_entries); // If we already have this key, just update its value if (newEntries.containsKey(key)) { newEntries[key] = value; - return OTelFactory.otelFactory!.traceState(newEntries); + return factory.traceState(newEntries); } // If adding a new key would exceed the limit, remove the oldest entry @@ -99,19 +99,16 @@ class TraceState { } newEntries[key] = value; - return OTelFactory.otelFactory!.traceState(newEntries); + return factory.traceState(newEntries); } /// Creates a new [TraceState] with the given [key] removed. TraceState remove(String key) { - if (OTelFactory.otelFactory == null) { - throw StateError('Call initialize() first.'); - } if (!_entries.containsKey(key)) return this; final newEntries = Map.from(_entries); newEntries.remove(key); - return OTelFactory.otelFactory!.traceState(newEntries); + return OTelFactory.getOrCreateDefault().traceState(newEntries); } /// Convert to W3C trace context header string diff --git a/test/unit/api/trace/trace_state_spec_compliance_test.dart b/test/unit/api/trace/trace_state_spec_compliance_test.dart new file mode 100644 index 0000000..69587a2 --- /dev/null +++ b/test/unit/api/trace/trace_state_spec_compliance_test.dart @@ -0,0 +1,65 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +import 'package:dartastic_opentelemetry_api/dartastic_opentelemetry_api.dart'; +import 'package:test/test.dart'; + +// Spec-compliance tests for TraceState (specification/trace/api.md): +// +// - "All mutating operations MUST return a new TraceState with the +// modifications applied." +// - "Every mutating operations MUST validate input parameters. If invalid +// value is passed the operation MUST NOT return TraceState containing +// invalid data and MUST follow the general error handling guidelines" — +// and error-handling.md: "API methods MUST NOT throw unhandled exceptions +// when used incorrectly by end users." Invalid input is rejected by +// ignoring it (log a warning), never by throwing. +void main() { + setUp(() { + OTelAPI.reset(); + OTelAPI.initialize( + endpoint: 'http://localhost:4318', + serviceName: 'test-service', + serviceVersion: '1.0.0', + ); + }); + + group('TraceState mutating operations never throw', () { + test('put with an invalid key returns the state unchanged', () { + final traceState = TraceState.fromMap({'vendor': 'value'}); + final result = traceState.put('INVALID KEY!', 'value'); + expect(result.entries, equals({'vendor': 'value'})); + }); + + test('put with an invalid value returns the state unchanged', () { + final traceState = TraceState.fromMap({'vendor': 'value'}); + final result = traceState.put('vendor2', 'bad,value'); + expect(result.entries, equals({'vendor': 'value'})); + }); + + test('put never returns TraceState containing invalid data', () { + final result = + TraceState.fromMap({'vendor': 'value'}).put('BAD KEY', 'v'); + for (final key in result.entries.keys) { + expect(TraceState.fromString('$key=${result.get(key)}').get(key), + isNotNull); + } + }); + }); + + group('TraceState works without an installed SDK', () { + test('put works after reset', () { + final traceState = TraceState.fromMap({'vendor': 'value'}); + OTelAPI.reset(); + final result = traceState.put('vendor2', 'value2'); + expect(result.get('vendor2'), equals('value2')); + }); + + test('remove works after reset', () { + final traceState = TraceState.fromMap({'vendor': 'value'}); + OTelAPI.reset(); + final result = traceState.remove('vendor'); + expect(result.get('vendor'), isNull); + }); + }); +} diff --git a/test/unit/api/trace/trace_state_test.dart b/test/unit/api/trace/trace_state_test.dart index c9505fb..12cee73 100644 --- a/test/unit/api/trace/trace_state_test.dart +++ b/test/unit/api/trace/trace_state_test.dart @@ -107,23 +107,25 @@ void main() { expect(identical(traceState, updatedState), isFalse); }); - test('rejects invalid key and value', () { + test('ignores invalid key and value instead of throwing', () { final traceState = OTelAPI.traceState({'vendora': 'value1'}); // Invalid key (uppercase) - expect(() => traceState.put('INVALID', 'value2'), throwsArgumentError); + expect(traceState.put('INVALID', 'value2').entries, + equals({'vendora': 'value1'})); // Invalid key (special char) - expect(() => traceState.put('invalid@', 'value2'), throwsArgumentError); + expect(traceState.put('invalid@', 'value2').entries, + equals({'vendora': 'value1'})); // Invalid value (contains invalid characters) - expect( - () => traceState.put('vendorb', 'value\u0000'), throwsArgumentError); + expect(traceState.put('vendorb', 'value\u0000').entries, + equals({'vendora': 'value1'})); // Invalid value (too long) final tooLongValue = 'x' * 257; - expect( - () => traceState.put('vendorb', tooLongValue), throwsArgumentError); + expect(traceState.put('vendorb', tooLongValue).entries, + equals({'vendora': 'value1'})); }); test('updates existing value', () { @@ -182,20 +184,19 @@ void main() { expect(state.get(key), equals('v')); }); - test('put rejects a tenant id longer than 241 chars', () { - expect(() => OTelAPI.traceState({}).put('${'a' * 242}@acme', 'v'), - throwsArgumentError); + test('put ignores a tenant id longer than 241 chars', () { + expect( + OTelAPI.traceState({}).put('${'a' * 242}@acme', 'v').isEmpty, isTrue); }); - test('put rejects a system id longer than 14 chars', () { - expect(() => OTelAPI.traceState({}).put('acme@${'a' * 15}', 'v'), - throwsArgumentError); + test('put ignores a system id longer than 14 chars', () { + expect( + OTelAPI.traceState({}).put('acme@${'a' * 15}', 'v').isEmpty, isTrue); }); - test('put rejects a system id that starts with a digit', () { + test('put ignores a system id that starts with a digit', () { // W3C system-id = lcalpha ... - expect(() => OTelAPI.traceState({}).put('acme@9vendor', 'v'), - throwsArgumentError); + expect(OTelAPI.traceState({}).put('acme@9vendor', 'v').isEmpty, isTrue); }); test('maintains immutability', () {