BrighterCommand / BrighterCommand/Brighter

Post-flavoured request schedules round-trip through System.Text.Json and silently corrupt readonly record structs

Open
#4,152 3 comments 0 reactions 1 assignee Claimed by @DevJonny View on GitHub
.NET 1 - Up Next Bug V10.X
Dominant language
C#
Stars
2.5k
Forks
296
Avg merge
1d 11h
Merged PRs (30d)
21

Description

> **Depends on:** #4149 (Newtonsoft compatibility), #4150 (RoutingKey/AMQP), #4151 (DictionaryKeyPolicy on bag).
>
> The fix proposed here will not deliver value until those three land, because the scheduler is otherwise unusable for any non-trivial request from a Newtonsoft-backed app — failures from those issues mask this one.

### Describe the bug

`AzureServiceBusScheduler.ScheduleAsync(TRequest, RequestSchedulerType.Post, ...)` serialises the request via `System.Text.Json` into an intermediate `FireAzureScheduler.RequestData` envelope. At fire time the consumer STJ-deserialises that envelope and then re-maps the request via the registered (Newtonsoft) `IAmAMessageMapper` for the wire.

For request types containing readonly record structs with explicit constructors that STJ cannot reconstruct cleanly, fields are silently zeroed. Concrete example: a value object

```csharp
public readonly record struct ClientActionId(ClientActionType Type, string Value);
```

round-trips with `Type = default(ClientActionType) = 0`. The bug is silent — no exception, just wrong data on the consumer.

The scheduler ignores the user-registered Newtonsoft mapper even when one is available, despite the fact that it must invoke that same mapper at fire time anyway.

### To Reproduce

1. Register a Newtonsoft-based `IAmAMessageMapper` and matching `Publication`.
2. Include a property on `MyRequest` whose type is a readonly record struct with an explicit constructor (any type STJ deserialises to `default`).
3. Schedule via `commandProcessor.SendAsync(TimeSpan, myRequest)`.
4. Observe the fired message: the affected field is `default`, not the value originally scheduled.

### Suggested fix

When a `Publication` and `IAmAMessageMapper` are registered for `TRequest`, skip the STJ `RequestData` round-trip entirely:

1. Map the request to a `Message` using the registered mapper.
2. Wrap that `Message` directly in `FireAzureScheduler.Message` (the same envelope path used by `Schedule(Message, ...)`).
3. Fall back to the current STJ `RequestData` flow only when no mapper is registered.

This guarantees the scheduler uses the same serialisation path the application configured for normal publishes, so anything that round-trips correctly on a normal `Post` will also round-trip correctly on a scheduled `Post`.

### Further technical details

- Brighter version: **10.4.1** (`Paramore.Brighter.MessageScheduler.Azure`)
- `dotnet --info`:
```
.NET SDK 10.0.108 (commit 94ea82652c, MSBuild 18.0.11+94ea82652)
Host 10.0.8 (x64)
RID: win-x64
```
- OS: Windows 11 (build 10.0.26200)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.