Messages without any headers do not have a headers map included into the basic.deliver frame

[jarpoole]

Describe the bug

RabbitMQ 3.13.x does not include the properties.headers key if there are no headers specified while RabbitMQ 3.12.x does. This impacts clients who may be making an assumption that they can do a direct property lookup on the properties.headers key for a specific header without first confirming if the headers key exists at all.

Reproduction steps

Not an expert in AMQP tooling but this is the simplest way I could think of to reproduce the discrepancy:

1. Start RabbitMQ 3.12.13 with docker run -p 15672:15672 rabbitmq:3.12.13-management-alpine
2. Navigate to http://localhost:15672 in your browser
3. Login to the management dashboard with username guest, password guest.
4. Click on the Queues and Streams tab, click Add a new queue at the bottom, give it a name like test and click Add queue
5. Click on the new test queue in the table
6. Click on publish message, provide a sample payload like "hello world" and click Publish message
7. Inspect element in the browser and navigate to the Network tab
8. Click Get messages and then click the Get Message(s) button
9. Select the get request that appears in the network tab, navigate to the response tab and observe that the properties.headers key is included with an empty object {} as the value

11. Repeat the same steps again with image rabbitmq:3.13.0-management-alpine and observe that this time there is no properties.headers key in the response.

Expected behavior

When consuming a message from a queue the properties.headers key should always be included even if no headers were specified when publishing.

Additional context

We ran into this issue because the type definitions for the typescript AMQP client we are using (amqplib) specify that the headers key will always be present. See https://github.com/DefinitelyTyped/DefinitelyTyped/blob/eef10d31a4119697aaf4a8a6b2511ba1e423ed43/types/amqplib/properties.d.ts#L179 and not that | undefined is NOT included for headers as it is for other keys.

[michaelklishin]

I'm not entirely sure why you expect the headers to be present for messages that have no headers. This does not make much sense in general. The fact that 3.12 worked that way does not mean much, there were many revisited decisions from the original years of RabbitMQ starting with at least 3.9.0.

The TypeScript library you are referring to can be updated to make the field an Option.

#5077 uses a more efficient format internally, by design.

Similarly, the HTTP API in 3.13 intentionally dropped some other rarely used fields in much more commonly used than this. This resulted in a 25% traffic decrease and lower CPU burn, because serializing a document with some 70 fields is not cheap, and if you can remove, say, 10 of them, that helps.

[michaelklishin]

For example, no client library that the RabbitMQ core team maintainers has made this decision to assume that headers are always present.

You can claim that that's because we still support older versions of Java and .NET, or some clients are for dynamically typed languages.

Well, there is an excellent Rust client that assumes that the headers map is optional.

The TypeScript client in question can decide to inject an empty map into deserialzied basic.deliver frames.

Why should RabbitMQ waste CPU cycles and bandwidth to do something that does not make much sense semantically, is not mentioned at all in the AMQP 0-9-1 spec, and is not needed or expected by many other client libraries?

Wouldn't it make more sense for the TypeScript client to insert an empty headers map, or switch to Option, whichever its maintainers would prefer?

[jarpoole]

On some level I kind of suspected the answer would be that this is not a bug and that the client lib needs to be updated so thank you for laying things out clearly so I can open a PR for @types/amqplib with justification!

[michaelklishin]

Thank you for bringing this up with enough details, and, of course, contributing to the ecosystem.

[michaelklishin]

Finally, none of the clients Team RabbitMQ maintainers needed any tweaking for #5077. The same client works just fine against a 3.7 node from years ago as it does against a 3.13.0 node.

This sounds like a client-specific behavior to me, and a very easy to address one. It's not like we have introduced a fundamentally new way of serializing deliveries, and forcing it down on every client maintainer.

They have decided to assume that headers are always present, history has proven them wrong. So what, it's a trivial change anyway, to the public API or to their implementation.

[michaelklishin]

I am afraid I am very much not looking forward for another debate on how realistic the "SemVer dream" is for a large enough project that supports N protocols and where hundreds of things that affect 0.1% of the user base can be considered "a breaking change."

PostgreSQL 16.2 (so, a minor release) requires their users to explicitly reindex some data to get the expected behavior of a certain index type. Are they following SemVer or not?

Very few larger projects can avoid any breaking changes of any kind ("API" can be defined very broadly) in minor releases. In particular if they have certain contractual release lifecycle expectations tied to majors and minors.

We try to break as few things as practically possible. This is why feature flags exist, why even some bug fixes sometimes end up behind feature flags, and why a client from 2009 or so (yes, from 15 years ago) can still connect and perform all key operations.

[jarpoole]

Yep this is fair. I will be recalibrating my mental model. Just thought it might be worth making explicit for other newcomers like me.

[kjnilsson]

@jarpoole the representation of the headers is valid according to the AMQP 0.9.1 specification so there is no SemVer violation here anyway that I can see. The bug in the rust client would have cause a problem at some point anyway, in 3.12 as well as another client lib may have posted a message without headers that the rust client would have failed to consume.

That said we are going to address the fact that a message with an empty headers map will now return the empty headers map as before. A message with an unset headers field will return an unset headers field.

[ansd]

#10623 fixes this issue.

[jarpoole]

Thanks everyone! I also got a patch to the Typescript types for amqplib merged to better align with the AMQP 0.9.1 spec as @kjnilsson mentioned (DefinitelyTyped/DefinitelyTyped#68773)

Surprisingly because that repo tries to keep the major versions of its @types packages in line with the underlying libs the PR bot published the new version as a patch not a major. Hopefully the error with typing that other projects will now see helps them get away from this non-standard behavior also!