Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #361: AttributeError in mailgun webhooks #363

Merged
merged 4 commits into from
Mar 5, 2024
Merged

Conversation

izimobil
Copy link
Contributor

@izimobil izimobil commented Mar 5, 2024

This fixes the case of delivery-status being None in the event data posted to mailgun webhook handler.
See: #361

This fixes the case of delivery-status being None in the event data
posted to mailgun webhook handler.
See: anymail#361
@izimobil
Copy link
Contributor Author

izimobil commented Mar 5, 2024

Hi, I realize I can reduce the raw_event in the test case to:

raw_event = mailgun_sign_payload(
    {
        "event-data": {
             "event": "accepted",
             "delivery-status": None,
        }
    }
)

Do you want me to update the PR ?

@medmunds
Copy link
Contributor

medmunds commented Mar 5, 2024

Thanks for this. Yes, the shorter test case would probably be clearer. (Mailgun was one of Anymail's first integrations, so the existing tests tend to be more verbose than necessary.)

"delivery-status": null doesn't show up in Mailgun's event documentation. I wonder if there are any other undocumented changes to their webhook payloads?

@izimobil
Copy link
Contributor Author

izimobil commented Mar 5, 2024

Thanks for this. Yes, the shorter test case would probably be clearer. (Mailgun was one of Anymail's first integrations, so the existing tests tend to be more verbose than necessary.)

OK !

"delivery-status": null doesn't show up in Mailgun's event documentation. I wonder if there are any other undocumented changes to their webhook payloads?

Well, to be honest, I don't know, we have a multi-tenant app, and the error showed up on a single tenant, probably a tenant with some weird MTA ?! I'll report the issue to mailgun !
Cheers

izimobil and others added 3 commits March 5, 2024 22:15
- shortened testcase payload to minimum required
- renamed method to be more consistant with existing code
@medmunds medmunds merged commit 5949069 into anymail:main Mar 5, 2024
58 checks passed
@medmunds
Copy link
Contributor

(Released in Anymail 10.3)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants