Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

NullReferenceException when accessing message.Size property #648

Closed
dolly22 opened this issue Feb 19, 2019 · 6 comments
Closed

NullReferenceException when accessing message.Size property #648

dolly22 opened this issue Feb 19, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@dolly22
Copy link

dolly22 commented Feb 19, 2019

Actual Behavior

When accessing empty message (message.Body is null) size property NullReferenceException is thrown.

Expected Behavior

Getting NullReferenceException was pretty unexpected. I would expect to get either 0 size or size of message properties

Versions

nuget 3.3.0 / netstandard2

@SeanFeldman
Copy link
Collaborator

Technically, the body can be either null or an empty array. Returning zero for both use cases would be wrong.

@dolly22 if your code needs to handle null body messages, shouldn't it then even bother with the Size property? Could you please share the code where you have a null body and need to use Size? I'd like to understand better what's happening from a usage perspective. Thanks.

@SeanFeldman
Copy link
Collaborator

@dolly22 ping

@dolly22
Copy link
Author

dolly22 commented Mar 4, 2019

I'm accessing Size property in MessageHandler callback for the purpose of some diagnostic logging (create logger scope with MessageId context, log basic message properties - id, retry, created, size, contenttype, ...)

So i have some pretty innocent looking code like this...

using (logger.BeginScope(new Dictionary<string, string>()
{
    ["MessageId"] = message.MessageId,
}))
{
    logger.LogDebug($"[{context.EntityPath}, id: {message.MessageId}, contentType: '{message.ContentType}'] start processing, try #{message.SystemProperties.DeliveryCount}, label '{message.Label}', size {message.Size} bytes, enqueued {message.SystemProperties.EnqueuedTimeUtc}");
    ...
}

that throws NullReferenceException when message body is null.

I also use some abstraction on top of message sender/handler for large messages. For such message service bus message body is null and content is stored as azure blob (only blob SAS uri is stored in message UserProperties).

@SeanFeldman
Copy link
Collaborator

I also use some abstraction on top of message sender/handler for large messages. For such message service bus message body is null and content is stored as azure blob (only blob SAS uri is stored in message UserProperties).

When it comes to abstraction like the one you've mentioned, it should play by the rules of the ASB client. Which is expecting a message sent out to be delivered and received with the contents. Sounds like the abstraction you're using is not doing that, causing message body to be null and therefore breaking the assumption client is making.

Are you using ServiceBus.AttachmentPlugin by any chance? If you are, it's a plugin that should be registered on both, sending and receiving ends, ensuring the content of the message is rehydrated.

That's why I think there's something off about this request. The entire message is either a valid message, or nothing (null is a possible value when receiving a message using MessageReceiver.ReceiveAsync().

@dolly22
Copy link
Author

dolly22 commented Mar 5, 2019

I'm not using AttachmentPlugin but some inhouse abstraction that uses streams to handle large content so message.Body byte[] is completely ignored when sending/receiving such messages.

I was supprised by this behaviour when migrating code from Microsoft.ServiceBus.Messaging / BrokeredMessage where this was completely ok. I can easily adapt/workaround to this. I'm just reporting this was pretty unexpected from someone who used this library.

For me it's supprising that this code throws NullReferenceException...

var msg = new Message();
Console.WriteLine(msg.Size);

yet it's completely valid to create/send/receive such message, just invalid to access it's Size. If this is expected than I (as library consumer) would prefer Size property not to exist at all and be forced to use msg.Body.Length directly.

@SeanFeldman
Copy link
Collaborator

If this is expected than I (as library consumer) would prefer Size property not to exist at all and be forced to use msg.Body.Length directly.

In case msg.Body is null, you'd still need to check rather than access .Length directly.

I'm looking at it from a different perspective. The old library returned zero when no body was provided. In that way, this is a regression from the expected behaviour when upgrading from the old client. Based on that, this would be categorized as a bug.

@SeanFeldman SeanFeldman added the bug label Mar 6, 2019
@SeanFeldman SeanFeldman added this to the 3.3.1 milestone Mar 6, 2019
@SeanFeldman SeanFeldman self-assigned this Mar 6, 2019
SeanFeldman added a commit to SeanFeldman/azure-service-bus-dotnet that referenced this issue Mar 6, 2019
@SeanFeldman SeanFeldman modified the milestones: 3.3.1, 3.4.0 Mar 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants