Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/common.props
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
<NoWarn>$(NoWarn);CS1591;NU1701</NoWarn>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<LangVersion>latest</LangVersion>
<Nullable>enable</Nullable>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
Comment on lines 16 to 20
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description mentions replacing <WarningsAsErrors> with <TreatWarningsAsErrors>, but this repo doesn’t appear to contain <WarningsAsErrors> anywhere, and TreatWarningsAsErrors is already present here. Please update the PR description (or include the missing change) so it accurately reflects what’s actually being modified (e.g., enabling <Nullable>enable</Nullable> in this file).

Copilot uses AI. Check for mistakes.
<PackageOutputPath>$(SolutionDir)artifacts</PackageOutputPath>
<PackageIcon>foundatio-icon.png</PackageIcon>
Expand Down
16 changes: 14 additions & 2 deletions samples/Foundatio.AzureStorage.Dequeue/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@
var mode = parseResult.GetValue(modeOption);
var count = parseResult.GetValue(countOption);

if (string.IsNullOrWhiteSpace(connectionString))
{
Console.Error.WriteLine("Error: Connection string is required. Use --connection-string or set AZURE_STORAGE_CONNECTION_STRING.");
return 1;
}

if (string.IsNullOrWhiteSpace(queueName))
{
Console.Error.WriteLine("Error: Queue name is required. Use --queue.");
return 1;
}

Console.WriteLine($"Using connection: {(connectionString == EmulatorConnectionString ? "Azure Storage Emulator" : "Custom connection string")}");
Console.WriteLine($"Mode: {mode}");
Console.WriteLine($"Queue: {queueName}");
Expand Down Expand Up @@ -120,14 +132,14 @@ static async Task DequeueMessages(string connectionString, string queueName, Azu
processed++;

logger.LogInformation("Dequeued message {MessageId}: '{Message}' from '{Source}' at {Timestamp}",
entry.Id, entry.Value.Message, entry.Value.Source, entry.Value.Timestamp);
entry.Id, entry.Value?.Message, entry.Value?.Source, entry.Value?.Timestamp);

logger.LogInformation(" CorrelationId: '{CorrelationId}'", entry.CorrelationId ?? "<none>");

if (entry.Properties != null && entry.Properties.Count > 0)
{
logger.LogInformation(" Properties: [{Properties}]",
string.Join(", ", entry.Properties.Select(p => $"{p.Key}={p.Value}")));
String.Join(", ", entry.Properties.Select(p => $"{p.Key}={p.Value}")));
}
else
{
Expand Down
41 changes: 29 additions & 12 deletions samples/Foundatio.AzureStorage.Enqueue/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,28 @@
var queueName = parseResult.GetValue(queueOption);
var message = parseResult.GetValue(messageOption);
var correlationId = parseResult.GetValue(correlationIdOption);
var properties = parseResult.GetValue(propertiesOption);
var properties = parseResult.GetValue(propertiesOption) ?? [];
var mode = parseResult.GetValue(modeOption);
var count = parseResult.GetValue(countOption);

if (string.IsNullOrWhiteSpace(connectionString))
{
Console.Error.WriteLine("Error: Connection string is required. Use --connection-string or set AZURE_STORAGE_CONNECTION_STRING.");
return 1;
}

if (string.IsNullOrWhiteSpace(queueName))
{
Console.Error.WriteLine("Error: Queue name is required. Use --queue.");
return 1;
}

if (string.IsNullOrWhiteSpace(message))
{
Console.Error.WriteLine("Error: Message is required. Use --message.");
return 1;
}

Console.WriteLine($"Using connection: {(connectionString == EmulatorConnectionString ? "Azure Storage Emulator" : "Custom connection string")}");
Console.WriteLine($"Mode: {mode}");
Console.WriteLine();
Expand All @@ -86,7 +104,7 @@
// Parse and invoke
return await rootCommand.Parse(args).InvokeAsync();

static async Task EnqueueMessages(string connectionString, string queueName, string message, string correlationId, string[] properties, AzureStorageQueueCompatibilityMode mode, int count)
static async Task EnqueueMessages(string connectionString, string queueName, string message, string? correlationId, string[] properties, AzureStorageQueueCompatibilityMode mode, int count)
{
using var loggerFactory = LoggerFactory.Create(builder => builder.AddConsole().SetMinimumLevel(LogLevel.Information));
var logger = loggerFactory.CreateLogger("Enqueue");
Expand All @@ -100,15 +118,12 @@ static async Task EnqueueMessages(string connectionString, string queueName, str
.LoggerFactory(loggerFactory));

var queueProperties = new Dictionary<string, string>();
if (properties != null)
foreach (var prop in properties)
{
foreach (var prop in properties)
var parts = prop.Split('=', 2);
if (parts.Length == 2)
{
var parts = prop.Split('=', 2);
if (parts.Length == 2)
{
queueProperties[parts[0]] = parts[1];
}
queueProperties[parts[0]] = parts[1];
}
}

Expand All @@ -122,15 +137,17 @@ static async Task EnqueueMessages(string connectionString, string queueName, str

var entryOptions = new QueueEntryOptions
{
CorrelationId = correlationId,
Properties = queueProperties.Count > 0 ? queueProperties : null
CorrelationId = correlationId
};

if (queueProperties.Count > 0)
entryOptions.Properties = queueProperties;

var messageId = await queue.EnqueueAsync(sampleMessage, entryOptions);

logger.LogInformation("Enqueued message {MessageId}: '{Message}' with CorrelationId: '{CorrelationId}' Properties: [{Properties}]",
messageId, sampleMessage.Message, correlationId ?? "<none>",
string.Join(", ", queueProperties.Select(p => $"{p.Key}={p.Value}")));
String.Join(", ", queueProperties.Select(p => $"{p.Key}={p.Value}")));
}

logger.LogInformation("Successfully enqueued {Count} message(s)", count);
Expand Down
4 changes: 2 additions & 2 deletions samples/Foundatio.AzureStorage.Enqueue/SampleMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace Foundatio.AzureStorage.Samples;

public record SampleMessage
{
public string Message { get; init; } = string.Empty;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like these should be made required and we should not be using String.Empty ever for defaults.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed: SampleMessage now uses
equired\ properties. No more \String.Empty\ defaults.

public required string Message { get; init; }
public DateTime Timestamp { get; init; } = DateTime.UtcNow;
public string Source { get; init; } = string.Empty;
public required string Source { get; init; }
}
2 changes: 1 addition & 1 deletion src/Foundatio.AzureStorage/Foundatio.AzureStorage.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<PackageReference Include="Azure.Storage.Blobs" Version="12.27.0" />
<PackageReference Include="Azure.Storage.Queues" Version="12.25.0" />

<PackageReference Include="Foundatio" Version="13.0.0-beta3.32" Condition="'$(ReferenceFoundatioSource)' == '' OR '$(ReferenceFoundatioSource)' == 'false'" />
<PackageReference Include="Foundatio" Version="13.0.0-beta3.41" Condition="'$(ReferenceFoundatioSource)' == '' OR '$(ReferenceFoundatioSource)' == 'false'" />
<ProjectReference Include="..\..\..\Foundatio\src\Foundatio\Foundatio.csproj" Condition="'$(ReferenceFoundatioSource)' == 'true'" />
</ItemGroup>
</Project>
43 changes: 28 additions & 15 deletions src/Foundatio.AzureStorage/Queues/AzureStorageQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ public class AzureStorageQueue<T> : QueueBase<T, AzureStorageQueueOptions<T>> wh

public AzureStorageQueue(AzureStorageQueueOptions<T> options) : base(options)
{
if (String.IsNullOrEmpty(options.ConnectionString))
throw new ArgumentException("ConnectionString is required.");
ArgumentException.ThrowIfNullOrWhiteSpace(options?.ConnectionString, nameof(options.ConnectionString));
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor validation uses options?.ConnectionString even though options is expected to be non-null (and is immediately dereferenced later). Using the null-conditional here can mask an unexpected null options (changing the failure mode) and is inconsistent with the AzureFileStorage constructor validation. Prefer validating options.ConnectionString directly after ensuring options is non-null.

Suggested change
ArgumentException.ThrowIfNullOrWhiteSpace(options?.ConnectionString, nameof(options.ConnectionString));
ArgumentNullException.ThrowIfNull(options);
ArgumentException.ThrowIfNullOrWhiteSpace(options.ConnectionString, nameof(options.ConnectionString));

Copilot uses AI. Check for mistakes.

var clientOptions = new QueueClientOptions();
#pragma warning disable CS0618 // Legacy mode is still supported internally for backward compatibility
Expand Down Expand Up @@ -71,7 +70,7 @@ await Task.WhenAll(
}
}

protected override async Task<string> EnqueueImplAsync(T data, QueueEntryOptions options)
protected override async Task<string?> EnqueueImplAsync(T data, QueueEntryOptions options)
{
if (!await OnEnqueuingAsync(data, options).AnyContext())
return null;
Expand Down Expand Up @@ -110,7 +109,7 @@ protected override async Task<string> EnqueueImplAsync(T data, QueueEntryOptions
return response.Value.MessageId;
}

protected override async Task<IQueueEntry<T>> DequeueImplAsync(CancellationToken linkedCancellationToken)
protected override async Task<IQueueEntry<T>?> DequeueImplAsync(CancellationToken linkedCancellationToken)
{
_logger.LogTrace("Checking for message: IsCancellationRequested={IsCancellationRequested} VisibilityTimeout={VisibilityTimeout}", linkedCancellationToken.IsCancellationRequested, _options.WorkItemTimeout);

Expand Down Expand Up @@ -177,9 +176,10 @@ protected override async Task<IQueueEntry<T>> DequeueImplAsync(CancellationToken
message.MessageId, insertedOn, nowUtc, queueTime.TotalMilliseconds, linkedCancellationToken.IsCancellationRequested);
Interlocked.Increment(ref _dequeuedCount);

T data;
string correlationId = null;
IDictionary<string, string> properties = null;
T? data;
string? correlationId = null;
IDictionary<string, string>? properties = null;
Exception? deserializeException = null;

try
{
Expand All @@ -189,9 +189,16 @@ protected override async Task<IQueueEntry<T>> DequeueImplAsync(CancellationToken
{
// Unwrap envelope to extract metadata
var envelope = _serializer.Deserialize<QueueMessageEnvelope<T>>(message.Body.ToArray());
data = envelope.Data;
correlationId = envelope.CorrelationId;
properties = envelope.Properties;
if (envelope is not null)
{
Comment thread
niemyjski marked this conversation as resolved.
data = envelope.Data;
correlationId = envelope.CorrelationId;
properties = envelope.Properties;
}
else
{
data = _serializer.Deserialize<T>(message.Body.ToArray());
}
}
catch (Exception ex)
{
Expand All @@ -209,7 +216,13 @@ protected override async Task<IQueueEntry<T>> DequeueImplAsync(CancellationToken
}
catch (Exception ex)
{
_logger.LogWarning(ex, "Error deserializing message {MessageId} (attempt {DequeueCount}), abandoning for retry", message.MessageId, message.DequeueCount);
data = null;
deserializeException = ex;
}

if (data is null)
{
_logger.LogWarning(deserializeException, "Error deserializing message {MessageId} (attempt {DequeueCount}), abandoning for retry", message.MessageId, message.DequeueCount);
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When data ends up null but no exception was thrown during deserialization (e.g., payload is literal null or an envelope deserializes with Data=null), deserializeException remains null and the log message still says "Error deserializing". Consider handling the deserializeException == null case separately (e.g., log that the payload was null) so operational logs accurately describe why the message is being abandoned/dead-lettered.

Suggested change
_logger.LogWarning(deserializeException, "Error deserializing message {MessageId} (attempt {DequeueCount}), abandoning for retry", message.MessageId, message.DequeueCount);
if (deserializeException is null)
_logger.LogWarning("Message {MessageId} deserialized to null payload (attempt {DequeueCount}), abandoning for retry", message.MessageId, message.DequeueCount);
else
_logger.LogWarning(deserializeException, "Error deserializing message {MessageId} (attempt {DequeueCount}), abandoning for retry", message.MessageId, message.DequeueCount);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is invalid feedback, it's fine to pass null for the exception here and it's what the logging api's do internally.


var poisonEntry = new AzureStorageQueueEntry<T>(message, null, null, null, this);
await AbandonAsync(poisonEntry).AnyContext();
Expand Down Expand Up @@ -419,7 +432,7 @@ protected override void StartWorkingImpl(Func<IQueueEntry<T>, CancellationToken,
{
_logger.LogTrace("WorkerLoop Signaled {QueueName}", _options.Name);

IQueueEntry<T> queueEntry = null;
IQueueEntry<T>? queueEntry = null;
try
{
queueEntry = await DequeueImplAsync(linkedCancellationToken.Token).AnyContext();
Expand Down Expand Up @@ -498,15 +511,15 @@ internal record QueueMessageEnvelope<T> where T : class
/// <summary>
/// Correlation ID for distributed tracing
/// </summary>
public string CorrelationId { get; init; }
public string? CorrelationId { get; init; }

/// <summary>
/// Custom properties/metadata
/// </summary>
public IDictionary<string, string> Properties { get; init; }
public IDictionary<string, string>? Properties { get; init; }

/// <summary>
/// The actual message payload
/// </summary>
public T Data { get; init; }
public required T Data { get; init; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class AzureStorageQueueEntry<T> : QueueEntry<T> where T : class
/// </summary>
public string PopReceipt { get; internal set; }

public AzureStorageQueueEntry(QueueMessage message, string correlationId, IDictionary<string, string> properties, T data, IQueue<T> queue)
public AzureStorageQueueEntry(QueueMessage message, string? correlationId, IDictionary<string, string>? properties, T? data, IQueue<T> queue)
: base(message.MessageId, correlationId, data, queue, message.InsertedOn?.UtcDateTime ?? DateTime.MinValue, (int)message.DequeueCount)
{
UnderlyingMessage = message;
Expand Down
6 changes: 3 additions & 3 deletions src/Foundatio.AzureStorage/Queues/AzureStorageQueueOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public enum AzureStorageQueueCompatibilityMode

public class AzureStorageQueueOptions<T> : SharedQueueOptions<T> where T : class
{
public string ConnectionString { get; set; }
public string? ConnectionString { get; set; }

/// <summary>
/// The interval to wait between polling for new messages when the queue is empty.
Expand Down Expand Up @@ -71,14 +71,14 @@ public class AzureStorageQueueOptions<T> : SharedQueueOptions<T> where T : class
/// };
/// </code>
/// </example>
public Action<RetryOptions> ConfigureRetry { get; set; }
public Action<RetryOptions>? ConfigureRetry { get; set; }
}

public class AzureStorageQueueOptionsBuilder<T> : SharedQueueOptionsBuilder<T, AzureStorageQueueOptions<T>, AzureStorageQueueOptionsBuilder<T>> where T : class
{
public AzureStorageQueueOptionsBuilder<T> ConnectionString(string connectionString)
{
ArgumentException.ThrowIfNullOrEmpty(connectionString);
ArgumentException.ThrowIfNullOrWhiteSpace(connectionString);

Target.ConnectionString = connectionString;
return this;
Expand Down
Loading
Loading