From 193fb809c8e583d1f17bd63fcbf52a72a3ecb840 Mon Sep 17 00:00:00 2001 From: Michael Bushe Date: Sat, 11 Jul 2026 08:20:33 +0200 Subject: [PATCH 1/2] test: pin TraceState spec contracts (5 currently failing) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit specification/trace/api.md: mutating operations MUST validate input, MUST NOT return TraceState containing invalid data, and MUST follow the error-handling guidelines — which forbid throwing on misuse. Current put() throws ArgumentError on invalid key/value, and put()/remove() throw StateError before initialization. Co-Authored-By: Claude Fable 5 --- .../trace_state_spec_compliance_test.dart | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 test/unit/api/trace/trace_state_spec_compliance_test.dart 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..08f9c77 --- /dev/null +++ b/test/unit/api/trace/trace_state_spec_compliance_test.dart @@ -0,0 +1,64 @@ +// Licensed under the Apache License, Version 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); + }); + }); +} From 6f94560fd1b2b2d78a8d15855903552b86050d82 Mon Sep 17 00:00:00 2001 From: Michael Bushe Date: Sat, 11 Jul 2026 08:28:27 +0200 Subject: [PATCH 2/2] fix: TraceState.put/remove ignore invalid input and never throw Turns the red spec tests green: - put() with an invalid key or value warns and returns the state unchanged instead of throwing ArgumentError, so TraceState never contains invalid data and never crashes the caller - put()/remove() use getOrCreateDefault so they work without an installed SDK Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 6 ++++ lib/src/api/trace/trace_state.dart | 17 ++++------ .../trace_state_spec_compliance_test.dart | 3 +- test/unit/api/trace/trace_state_test.dart | 33 ++++++++++--------- 4 files changed, 32 insertions(+), 27 deletions(-) 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 index 08f9c77..69587a2 100644 --- a/test/unit/api/trace/trace_state_spec_compliance_test.dart +++ b/test/unit/api/trace/trace_state_spec_compliance_test.dart @@ -1,4 +1,5 @@ -// Licensed under the Apache License, Version 2.0 +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 import 'package:dartastic_opentelemetry_api/dartastic_opentelemetry_api.dart'; import 'package:test/test.dart'; 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', () {