Skip to content

Conversation

@quezlatch
Copy link
Contributor

@quezlatch quezlatch commented Nov 8, 2025

User description

the azure service bus provider currently has an issue where meta data that is copied to service bus message attributes includes object types that are not serialisable by the azure sdk.

this pr filters out the meta data objects which are not serialisable.

this resolves #447


PR Type

Bug fix, Tests


Description

  • Filters non-serializable custom attributes from Service Bus messages

  • Adds IsSerialisableByServiceBus helper method to validate attribute types

  • Replaces null check with serialization validation in message builder

  • Includes comprehensive unit tests for serializable and non-serializable types


Diagram Walkthrough

flowchart LR
  A["Service Bus Message Builder"] -->|validates attributes| B["IsSerialisableByServiceBus Helper"]
  B -->|checks type| C["Allowed Types<br/>string, int, bool, Guid, etc."]
  B -->|rejects| D["Non-serializable Types<br/>objects, collections, delegates"]
  E["Unit Tests"] -->|verifies| B
Loading

File Walkthrough

Relevant files
Bug fix
ServiceBusMessageBuilder.cs
Replace null check with serialization validation                 

src/Azure/src/Eventuous.Azure.ServiceBus/Producers/ServiceBusMessageBuilder.cs

  • Reformatted code alignment for consistency
  • Replaced null check filter with IsSerialisableByServiceBus validation
  • Added static import of ServiceBusHelper for serialization checking
  • Ensures only serializable attributes are added to Service Bus messages
+22/-21 
Enhancement
ServiceBusHelper.cs
Add Service Bus serialization validation helper                   

src/Azure/src/Eventuous.Azure.ServiceBus/Shared/ServiceBusHelper.cs

  • New helper class with IsSerialisableByServiceBus static method
  • Validates if object types are serializable by Azure Service Bus SDK
  • Supports 18 serializable types including primitives, Guid, DateTime,
    Uri, Stream
  • Returns false for null and non-serializable complex types
+31/-0   
Tests
IsSerialisableByServiceBus.cs
Add serialization validation unit tests                                   

src/Azure/test/Eventuous.Tests.Azure.ServiceBus/IsSerialisableByServiceBus.cs

  • New test class with comprehensive test coverage for serialization
    validation
  • PassingTestData tests 17 serializable types including primitives and
    common types
  • FailingTestData tests 7 non-serializable types including null,
    objects, collections
  • Uses MethodDataSource for parameterized testing of both passing and
    failing cases
+45/-0   

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 8, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 02ff0ac)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟢
🎫 #447
🟢 -
F
i
x
s
e
r
a
l
z
t
o
n
c
p
w
h
u
b
g
v
A
S
B
G
y
P
d
H
M
m
j
f
.
,
E
/
D
K
R
Codebase Duplication Compliance
🟢
No codebase code duplication found New Components Detected (Top 5):
- ServiceBusHelper
- IsSerialisableByServiceBus
- IsSerialisableByServiceBus
- PassingTestData
- FailingTestData
Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit adba5d4
Security Compliance
Invalid property types

Description: The allowlist for serializable application property types includes Stream, which the Azure
Service Bus SDK does not support for application properties and could reintroduce
serialization errors if such values pass through.
ServiceBusHelper.cs [8-30]

Referred Code
public static bool IsSerialisableByServiceBus(object? value) => 
   value is not null && (
       value is string ||
       value is bool ||
       value is byte ||
       value is sbyte ||
       value is short ||
       value is ushort ||
       value is int ||
       value is uint ||
       value is long ||
       value is ulong ||
       value is float ||
       value is double ||
       value is decimal ||
       value is char ||
       value is Guid ||
       value is DateTime ||
       value is DateTimeOffset ||
       value is Stream ||
       value is Uri ||


 ... (clipped 2 lines)
Ticket Compliance
🟡
🎫 #447
🟢 Prevent putting non-serializable objects into Azure Service Bus message application
properties to avoid serialization exceptions.
Ensure only types supported by Azure Service Bus SDK are added as custom
headers/application properties.
Keep existing reserved/known attributes behavior intact while filtering problematic
metadata.
Provide a robust approach that works with Eventuous.Gateway and ServiceBus producer
without breaking other producers.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Logging Context: The new code modifies message property handling and filters metadata but does not add or
reference audit logging for critical actions, and it’s unclear from the diff whether such
actions are logged elsewhere.

Referred Code
internal ServiceBusMessage CreateServiceBusMessage(ProducedMessage message) {
    var (messageType, contentType, payload) = _serializer.SerializeEvent(message.Message);
    _setActivityMessageType?.Invoke(messageType);

    var metadata = message.Metadata;

    var serviceBusMessage = new ServiceBusMessage(payload) {
        ContentType = contentType,
        MessageId = metadata?.GetValueOrDefault(_attributes.MessageId, message.MessageId)?.ToString(),
        Subject = metadata?.GetValueOrDefault(_attributes.Subject, _options?.Subject)?.ToString(),
        TimeToLive = _options?.TimeToLive ?? TimeSpan.MaxValue,
        CorrelationId = message.Metadata?.GetCorrelationId(),
        To = metadata?.GetValueOrDefault(_attributes.To, _options?.To)?.ToString(),
        ReplyTo = metadata?.GetValueOrDefault(_attributes.ReplyTo, _options?.ReplyTo)?.ToString()
    };

    var reservedAttributes = _attributes.ReservedNames();
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Edge Cases: The helper filters non-serializable types but provides no error handling or logging for
rejected values, which may hinder debugging if critical metadata is dropped.

Referred Code
public static bool IsSerialisableByServiceBus(object? value) => 
   value is not null && (
       value is string ||
       value is bool ||
       value is byte ||
       value is sbyte ||
       value is short ||
       value is ushort ||
       value is int ||
       value is uint ||
       value is long ||
       value is ulong ||
       value is float ||
       value is double ||
       value is decimal ||
       value is char ||
       value is Guid ||
       value is DateTime ||
       value is DateTimeOffset ||
       value is Stream ||
       value is Uri ||


 ... (clipped 2 lines)
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation: The code filters metadata to Service Bus-supported types but otherwise trusts incoming
metadata and headers without additional validation or normalization, which may warrant
review depending on upstream sources.

Referred Code
    IEnumerable<KeyValuePair<string, object>> GetCustomApplicationProperties(ProducedMessage message, string messageType, HashSet<string> reservedAttributes)
        => (message.Metadata ?? [])
            .Concat(message.AdditionalHeaders ?? [])
            .Concat([new(_attributes.MessageType, messageType), new(_attributes.StreamName, _streamName)])
            .Where(pair => !reservedAttributes.Contains(pair.Key))
            .Where(pair => IsSerialisableByServiceBus(pair.Value))
            .Select(pair => new KeyValuePair<string, object>(pair.Key, pair.Value!)); 
}

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 8, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Remove unsupported Stream type check

Remove the value is Stream check from the IsSerialisableByServiceBus method, as
Stream is not a supported type for Azure Service Bus application properties.

src/Azure/src/Eventuous.Azure.ServiceBus/Shared/ServiceBusHelper.cs [8-30]

 public static bool IsSerialisableByServiceBus(object? value) => 
     value is not null && (
         value is string ||
         value is bool ||
         value is byte ||
         value is sbyte ||
         value is short ||
         value is ushort ||
         value is int ||
         value is uint ||
         value is long ||
         value is ulong ||
         value is float ||
         value is double ||
         value is decimal ||
         value is char ||
         value is Guid ||
         value is DateTime ||
         value is DateTimeOffset ||
-        value is Stream ||
         value is Uri ||
         value is TimeSpan
     );
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that Stream is not a supported type for Service Bus application properties, preventing a runtime exception and fixing a bug in the newly added helper method.

High
Fix incorrect async test method

Remove the async and await keywords from the Passes and Fails test methods and
change their return type to void to fix a compilation error.

src/Azure/test/Eventuous.Tests.Azure.ServiceBus/IsSerialisableByServiceBus.cs [38-44]

 [Test]
 [MethodDataSource(nameof(PassingTestData))]
-public async Task Passes(object value) => await Assert.That(IsSerialisableByServiceBus(value)).IsTrue();
+public void Passes(object value) => Assert.That(IsSerialisableByServiceBus(value)).IsTrue();
 
 [Test]
 [MethodDataSource(nameof(FailingTestData))]
-public async Task Fails(object value) => await Assert.That(IsSerialisableByServiceBus(value)).IsFalse();
+public void Fails(object value) => Assert.That(IsSerialisableByServiceBus(value)).IsFalse();

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a compilation error in the new test methods by pointing out the misuse of async and await on synchronous assertion calls, which is essential for the tests to run.

Medium
High-level
Consider serializing complex metadata values

Instead of filtering out and discarding metadata properties that are not
natively serializable by Azure Service Bus, consider serializing them to a
string format like JSON. This approach would prevent data loss while still
resolving the serialization crash.

Examples:

src/Azure/src/Eventuous.Azure.ServiceBus/Producers/ServiceBusMessageBuilder.cs [64-70]
    IEnumerable<KeyValuePair<string, object>> GetCustomApplicationProperties(ProducedMessage message, string messageType, HashSet<string> reservedAttributes)
        => (message.Metadata ?? [])
            .Concat(message.AdditionalHeaders ?? [])
            .Concat([new(_attributes.MessageType, messageType), new(_attributes.StreamName, _streamName)])
            .Where(pair => !reservedAttributes.Contains(pair.Key))
            .Where(pair => IsSerialisableByServiceBus(pair.Value))
            .Select(pair => new KeyValuePair<string, object>(pair.Key, pair.Value!)); 

Solution Walkthrough:

Before:

// In ServiceBusMessageBuilder.cs
IEnumerable<KeyValuePair<string, object>> GetCustomApplicationProperties(...)
    => (message.Metadata ?? [])
        .Concat(...)
        .Where(pair => !reservedAttributes.Contains(pair.Key))
        // This filters out and discards any value that is not a simple type.
        .Where(pair => IsSerialisableByServiceBus(pair.Value))
        .Select(pair => new KeyValuePair<string, object>(pair.Key, pair.Value!));

// In ServiceBusHelper.cs
public static bool IsSerialisableByServiceBus(object? value) {
    // returns true for string, int, bool, Guid, etc.
    // returns false for complex objects.
}

After:

// In ServiceBusMessageBuilder.cs
IEnumerable<KeyValuePair<string, object>> GetCustomApplicationProperties(...)
    => (message.Metadata ?? [])
        .Concat(...)
        .Where(pair => !reservedAttributes.Contains(pair.Key) && pair.Value is not null)
        .Select(pair => {
            var value = pair.Value!;
            if (IsSerialisableByServiceBus(value)) {
                return new KeyValuePair<string, object>(pair.Key, value);
            }
            // Serialize complex objects to a string (e.g., JSON) to preserve the data.
            var serializedValue = JsonSerializer.Serialize(value);
            return new KeyValuePair<string, object>(pair.Key, serializedValue);
        });
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the PR's fix causes data loss by filtering complex metadata, and proposes a more robust solution of serializing these objects to a string, which preserves information while still preventing the crash.

Medium
  • Update

@github-actions
Copy link

github-actions bot commented Nov 8, 2025

Test Results

 51 files  + 34   51 suites  +34   27m 47s ⏱️ + 18m 44s
262 tests + 30  262 ✅ + 30  0 💤 ±0  0 ❌ ±0 
789 runs  +552  789 ✅ +552  0 💤 ±0  0 ❌ ±0 

Results for commit 02ff0ac. ± Comparison against base commit 41fc89a.

This pull request removes 2 and adds 32 tests. Note that renamed tests count towards both.
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 4, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2025-11-08T19:05:05.9983340+00:00 })
Eventuous.Tests.Subscriptions.SequenceTests ‑ ShouldReturnFirstBefore(CommitPosition { Position: 0, Sequence: 1, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 6, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 8, Timestamp: 2025-11-08T19:05:05.9983340+00:00 }, CommitPosition { Position: 0, Sequence: 2, Timestamp: 2025-11-08T19:05:05.9983340+00:00 })
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails(Action)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails(Object)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails(System.Threading.Tasks.Task`1[System.Threading.Tasks.VoidTaskResult])
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails(WeakReference)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails([System.Object, v])
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails([k, System.Object])
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Fails(null)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(00:05:00)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(1)
Eventuous.Tests.Azure.ServiceBus.IsSerialisableByServiceBus ‑ Passes(1.23)
…

♻️ This comment has been updated with latest results.

@alexeyzimarev alexeyzimarev merged commit a5bb50c into Eventuous:dev Nov 12, 2025
10 of 13 checks passed
@quezlatch quezlatch deleted the filter-non-serialisable-attributes-from-service-bus-message branch November 14, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway with ServiceBusProducer don't work

2 participants