Skip to content

Conversation

@spookylukey
Copy link
Contributor

The comment in MIMEBase.get_payload() explains that it can return None when it is a multi-part message. In this case we need to convert the whole thing to bytes.

Without this patch, in production you get a crasher like the following:

  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/django/core/mail/message.py", line 342, in send
    return self.get_connection(fail_silently).send_messages([self])
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/anymail/backends/base.py", line 88, in send_messages
    sent = self._send(message)
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/anymail/backends/base_requests.py", line 56, in _send
    return super(AnymailRequestsBackend, self)._send(message)
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/anymail/backends/base.py", line 118, in _send
    response = self.post_to_esp(payload, message)
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/anymail/backends/base_requests.py", line 70, in post_to_esp
    except requests.RequestException as err:
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/requests/sessions.py", line 474, in request
    prep = self.prepare_request(req)
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/requests/sessions.py", line 407, in prepare_request
    hooks=merge_hooks(request.hooks, self.hooks),
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/requests/models.py", line 305, in prepare
    self.prepare_body(data, files, json)
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/requests/models.py", line 499, in prepare_body
    (body, content_type) = self._encode_files(files, data)
  File "/home/luke/.virtualenvs/cciw/lib/python3.5/site-packages/requests/models.py", line 158, in _encode_files
    fdata = fp.read()
AttributeError: 'NoneType' object has no attribute 'read'

This exception doesn't show up so easily in the test because the requests calls are mocked out.

@medmunds
Copy link
Contributor

Thanks for the find and fix.

This exception doesn't show up so easily in the test because the requests calls are mocked out.

This has come up before with some other bugs. There are integration tests that go all the way through requests to the actual ESP, but they're not as easy to run. (And tend to group everything together into a small number of tests, to preserve free ESP credits.)

I would like to find some way for the normal tests to mock requests much deeper in the process, but haven't figured out how to do that without really depending on undocumented requests internals. Probably time to look at that again.

@medmunds medmunds merged commit 7d74480 into anymail:master Apr 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants