BrighterCommand / BrighterCommand/Brighter

Four configuration members that are declared but not honoured, or documented with the wrong value

Open
#4,296 0 comments 0 reactions 0 assignees View on GitHub
.NET 0 - Backlog Bug
Dominant language
C#
Stars
2.5k
Forks
296
Avg merge
1d 11h
Merged PRs (30d)
21

Description

Found while writing the configuration reference tables for the [Docs](https://github.com/BrighterCommand/Docs)
repo (spec 012), where every table is written from the type rather than from existing prose.
Four small findings, all in `src/`, grouped because they are the same class of defect: **a member
a reader can set, or a documented value a reader can rely on, that the code does not honour.**

Everything below is at tag `10.7.0` and re-checked at `origin/master` unless stated. Each
"nothing reads this" claim was run with a control, because a grep that finds nothing looks
exactly like a grep pointed at the wrong thing — the controls are given inline.

---

### 1. `DynamoDbConfiguration.Timeout` is assigned and never read

`src/Paramore.Brighter.Outbox.DynamoDB/DynamoDbConfiguration.cs` (and `.V4`, identical):

```csharp
/// Timeout in milliseconds
public int Timeout { get; }
// ...
public DynamoDbConfiguration(string? tableName = null, int timeout = 500, int numberOfShards = 3, int scanConcurrency = 3)
{
Timeout = timeout; // the only write; there is no read
}
```

`Timeout` appears three times per file — doc comment, declaration, assignment — and nowhere else
in any `src/` file that references `DynamoDbConfiguration`. **Control:** `TableName` on the same
type is read 41 times across `src/`.

So `new DynamoDbConfiguration(timeout: 50)` compiles, reads back, and changes nothing.

### 2. …and `DynamoDbOutbox` ignores the `outBoxTimeout` argument as well

Worth stating beside (1), because it is the place a reader would look next. Every occurrence of
`outBoxTimeout` / `outboxTimeout` in `DynamoDbOutbox.cs` is a doc comment, a parameter
declaration, or a pass-through from a sync overload to its async twin. The terminal `AddAsync`,
`GetAsync` and `DispatchedMessagesAsync` bodies never read it — `AddAsync` goes straight from
`GetShardNumber` to `WriteMessageToOutbox`.

The doc comment on each says *"Timeout in milliseconds; -1 for default timeout"*, which promises
a behaviour that is not there. Whether the fix is to honour it or to document the AWS SDK client
as the only timeout that applies is your call — I have documented the latter for now.

### 3. `DynamoDbInboxConfiguration.Credentials` and `.Region` are read by nothing

`DynamoDbInbox` reads exactly one member of its configuration, `_configuration.TableName`
(3 occurrences). `Credentials` and `Region` appear only in their own declarations.

The two packages differ, which makes the AWS SDK v4 case the odder one:

| Package | Declaration | Consequence |
|---|---|---|
| `Paramore.Brighter.Inbox.DynamoDB` | `{ get; }`, never assigned — the primary constructor takes only `tableName` | permanently `null`; a caller cannot even set them |
| `Paramore.Brighter.Inbox.DynamoDB.V4` | `{ get; set; }` | settable, and still read by nothing |

Note this is the residue of #2837, which observed that `DynamoDbInboxConfiguration` "exists but
there doesn't seem to be any references" and was closed COMPLETED. `TableName` is wired up now;
these two were not.

### 4. `Publication.Type`'s doc comment states a default the code does not set

`src/Paramore.Brighter/Publication.cs`:

```csharp
/// Default: "goparamore.io.Paramore.Brighter.Message" for backward compatibility as required
public CloudEventsType Type { get; set; } = CloudEventsType.Empty; // = new(string.Empty)
```

The string `goparamore.io.Paramore.Brighter.Message` appears nowhere in `src/` except that
comment (and an unrelated example comment in `RedisMessageCreator`). `publication.Type` is
consumed as written — `Type = publication.Type?.Value ?? string.Empty` in
`CloudEventJsonMessageMapper` — so a publication with no `Type` set emits an **empty** CloudEvents
`type`, not the legacy value.

The *"for backward compatibility as required"* clause reads like an intent that was not
implemented, so I cannot tell whether the comment or the code is the thing to change.

### 5. `RocketMqPublication.Instrumentation` carries `Tag`'s doc comment verbatim

`src/Paramore.Brighter.MessagingGateway.RocketMQ/RocketMqPublication.cs`: the ``,
`` and `` above `public InstrumentationOptions? Instrumentation { get; set; }`
are a copy of the ones above `Tag` — they describe message tags, tag naming conventions and
consumer-side tag filtering. So the XML documentation for the telemetry option describes a
different feature entirely, and that is what IntelliSense shows.

---

### Why these are grouped

None is urgent and none breaks anything at runtime. They share a failure mode that documentation
cannot fix from the outside: **a reader who sets `Timeout`, sets `Region`, or relies on the
documented `Type` default gets no error, no warning, and no effect.** (1) and (3) also make it
impossible to write an honest options table — a table listing the member is wrong about what it
does, and a table omitting it is wrong about what exists — which is how they were found.

Happy to open a PR for any of them, though (2) and (4) look like decisions rather than
mechanical fixes.

Contributor guide

Open the contributing guide

Research direction

Start with the named configuration and publication files in src/, especially DynamoDbConfiguration.cs, DynamoDbOutbox.cs, DynamoDbInboxConfiguration, Publication.cs, and RocketMqPublication.cs. Trace each reported member from declaration through its consumers, including CloudEventJsonMessageMapper, and confirm the intended behavior before changing anything. Done means every listed option or documented default either has matching behavior and documentation, or has an explicit resolved decision.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.