better 413 response handling #2418
Conversation
| if self.too_big_count: | ||
| if self.too_big_count > 1: | ||
| lines += [ | ||
| "%d messages were rejected by the server as too big." % self.too_big_count |
There was a problem hiding this comment.
To stay consistent with the rest of the info page, I'd prefer this message to be displayed with the other stats on the transactions, i.e. something like:
Transactions rejected (too large): 15
right below Transactions flushed. What do you think?
There was a problem hiding this comment.
That sounds good to me. Should it appear there even if none have been rejected?
|
@gmmeyer Cool stuff! Added in 2 comments |
| @@ -242,6 +242,7 @@ <h1 id="status-header">Agent info</h1> | |||
| <dt>Flush count</dt> <dd>{{ forwarder['flush_count'] }}</dd> | |||
| <dt>Queue size</dt> <dd>{{ forwarder['queue_size'] }} bytes</dd> | |||
| <dt>Queue length</dt> <dd>{{ forwarder['queue_length'] }}</dd> | |||
| <dt>Transactions Rejected (too large)</dt> <dd>{{forwarder['too_big_count']}}</dd> | |||
There was a problem hiding this comment.
nitpick: could you add spaces between {{, }} and the var that's displayed (just for consistency)
Also, it's unrelated but while we're at it: could you add the Transactions received and Transactions flushed stats on this page too. Thanks!
There was a problem hiding this comment.
Yup, I can do all of that.
gmmeyer
force-pushed
the
413-response
branch
2 times, most recently
from
7a070e7 to
8381f09
Compare
| @@ -757,7 +757,7 @@ class ForwarderStatus(AgentStatus): | |||
| NAME = 'Forwarder' | |||
|
|
|||
| def __init__(self, queue_length=0, queue_size=0, flush_count=0, transactions_received=0, | |||
| transactions_flushed=0): | |||
| transactions_flushed=0, too_big_count=0): | |||
There was a problem hiding this comment.
nitpick: any reason for changing the indentation here? :)
There was a problem hiding this comment.
I don't think so. I'll change it back. I think it was just what my autoindent did.
|
One last nitpcik, otherwise looks gtg! |
|
👍 Feel free to squash your commits and then merge 🎉 |
When propjoe responds with a 413 error (payload too big), the agent used to retry the request. It shouldn't. The request shouldn't ever succeed.
This stops it from resubmitting the request. It logs a warning, saying what happened. It also adds a warning to the info page that a request was rejected.