SES: Fix sent email counter double incrementing#7919
Conversation
bentsku
left a comment
There was a problem hiding this comment.
I have one question regarding maybe something out of scope of this PR.
If this is too out of scope, I guess we could simplify the test to not have Destinations recipients and only one header To:? Or the same example as in the issue, one Destinations and no headers.
And create an xfailed test for the wrong behaviour of moto.
tests/integration/test_ses.py
Outdated
| ) | ||
|
|
||
| new_counter = ses_client.get_send_quota()["SentLast24Hours"] | ||
| assert new_counter == counter + 2 |
There was a problem hiding this comment.
question: I am confused, wasn't the issue report that the counter should be only +1? Or am I missing something?
After checking in moto, it seems moto is adding recipients from the raw headers to the sum. However, what if the recipient in Destinations and in the headers is the same? It seems they would be counted twice. (this previous part was before finding out the answer in the post under).
Also, after finding an old bug report in LocalStack and this SO post:
#6287
https://stackoverflow.com/questions/27446357/how-to-add-cc-and-bcc-in-amazon-ses-sendrawemail
It seems that if we have Destinations set then we shouldn't take into account the headers. So the count should be +1 here?
There was a problem hiding this comment.
Good catch! I confirmed this against prod AWS. This will need fixes both in LS and Moto and a out of scope of this issue. For now I've updated the test and added a fixme in the SES provider
bentsku
left a comment
There was a problem hiding this comment.
LGTM! Thanks for adding the note 👍
Remove Moto operation being invoked twice which caused sent email counter to be double incremented.
Fixes #7902