amqp-node / amqplib

AMQP 0-9-1 library and client for Node.JS
https://amqp-node.github.io/amqplib/
Other
3.69k stars 474 forks source link

"Frame size exceeds frame max" (RabbitMQ 3.7.7) #462

Closed floyd-may closed 5 years ago

floyd-may commented 6 years ago

Appears to be related to https://github.com/squaremo/amqp.node/issues/135. Stack trace appears to be pretty similar as well. Creating a new issue because the x-death header issue is not in play here as it appeared to be in the other issue. I'm attempting to read messages from an error queue that MassTransit creates when processing a message throws an exception.

Error: Frame size exceeds frame max
    at parseFrame (C:\Src\ft-queue-reader\node_modules\amqplib\lib\frame.js:56:13)
    at Connection.C.recvFrame (C:\Src\ft-queue-reader\node_modules\amqplib\lib\connection.js:604:15)
    at Socket.go (C:\Src\ft-queue-reader\node_modules\amqplib\lib\connection.js:477:30)
    at emitNone (events.js:105:13)
    at Socket.emit (events.js:207:7)
    at emitReadable_ (_stream_readable.js:514:10)
    at emitReadable (_stream_readable.js:508:7)
    at addChunk (_stream_readable.js:275:7)
    at readableAddChunk (_stream_readable.js:251:11)
    at Socket.Readable.push (_stream_readable.js:209:10)
    at TCP.onread (net.js:587:20)
squaremo commented 6 years ago

Is MassTransit adding any headers to the messages?

floyd-may commented 6 years ago

It most certainly is. Am I mistaken to believe that shouldn't be a protocol violation?

(misspelled from my mobile phone)

On Thu, Oct 4, 2018, 1:28 PM Michael Bridgen notifications@github.com wrote:

Is MassTransit adding any headers to the messages?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/squaremo/amqp.node/issues/462#issuecomment-427122799, or mute the thread https://github.com/notifications/unsubscribe-auth/ABiMNMxMytXb9Yg20go22kqHidsy47U-ks5uhlNcgaJpZM4XGWBK .

squaremo commented 6 years ago

It most certainly is. Am I mistaken to believe that shouldn't be a protocol violation?

Adding headers is OK yeah. The spec says nothing about what to do if the message header frame is too big to encode into the maximum frame size -- there's no way to have a continuation (header) frame, for example.

The default maximum frame size in amqplib is 4k bytes. That's pretty generous, but it's certainly possible that a careless intermediary could add ~4k of headers. Bumping the max size up with the connection argument frameMax might fix it. But there's the possibility that the added headers are unbounded (as they were in #135).

floyd-may commented 6 years ago

Here's some code from the client library that MassTransit uses:

https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/master/projects/client/RabbitMQ.Client/src/client/impl/Frame.cs

If I'm reading this correctly, the FIXME comment here on line 237 is where amqp.node is throwing. Based on the spec, I think amqp.node is "righter" to throw here:

Agreed limits MAY enable both parties to pre-allocate key buffers, avoiding deadlocks. Every incoming frame either obeys the agreed limits, and so is "safe", or exceeds them, in which case the other party IS faulty and MUST be disconnected. This is very much in keeping with the “it either works properly or it doesn't work at all” philosophy of AMQP.

It seems like the C# client is being more "tolerant" but feels guilty about it.

However, what I don't have (and probably am out of my depth in getting) is the details of the negotiation. What would you recommend?

squaremo commented 6 years ago

However, what I don't have (and probably am out of my depth in getting) is the details of the negotiation. What would you recommend?

There's a negotiation in the connection opening epic, governed by the Tune and TuneOK; the server sends its preferred value in Tune, and it can be lowered or accepted by the client in TuneOK. RabbitMQ sends either 0 (meaning unlimited) or a large value, I can't remember which; amqplib, by default, lowers it to 4k, a number I decided on unscientifically as being better for multiplexing fairness. You can set the value you want with the connection parameter frameMax, either in the querystring of the URL or field of the object, including to 0 meaning "accept whatever the server says".

cressie176 commented 5 years ago

@floyd-may ok to close?

floyd-may commented 5 years ago

Sure? ¯\(ツ)/¯ Seems like amqplib is doing The Right Thing ™ here.

francardoso93 commented 3 years ago

Do you guys have a suggestion of how can I discard the message with the large headers and keep consumer going? (Or recreating consumer)