Page MenuHomePhabricator

When we fail to process mail, tell the user about it
ClosedPublic

Authored by epriestley on Apr 3 2014, 9:52 PM.
Tags
None
Referenced Files
F13190513: D8692.diff
Sat, May 11, 11:37 AM
Unknown Object (File)
Fri, May 3, 8:31 AM
Unknown Object (File)
Mon, Apr 29, 2:10 AM
Unknown Object (File)
Sun, Apr 28, 12:07 PM
Unknown Object (File)
Sun, Apr 28, 12:07 PM
Unknown Object (File)
Sun, Apr 28, 12:07 PM
Unknown Object (File)
Sun, Apr 28, 12:07 PM
Unknown Object (File)
Sun, Apr 28, 12:07 PM
Subscribers

Details

Summary

Ref T4371. Ref T4699. Fixes T3994.

Currently, we're very conservative about sending errors back to users. A concern I had about this was that mistakes could lead to email loops, massive amounts of email spam, etc. Because of this, I was pretty hesitant about replying to email with more email when I wrote this stuff.

However, this was a long time ago. We now have Message-ID deduplication, "X-Phabricator-Sent-This-Mail", generally better mail infrastructure, and rate limiting. Together, these mechanisms should reasonably prevent anything crazy (primarily, infinite email loops) from happening.

Thus:

  • When we hit any processing error after receiving a mail, try to send the author a reply with details about what went wrong. These are limited to 6 per hour per address.
  • Rewrite most of the errors to be more detailed and informative.
  • Rewrite most of the errors in a user-facing voice ("You sent this mail..." instead of "This mail was sent..").
  • Remove the redundant, less sophisticated code which does something similar in Differential.
Test Plan
  • Using scripts/mail/mail_receiver.php, artificially received a pile of mail.
  • Hit a bunch of different errors.
  • Saw reasonable error mail get sent to me.
  • Saw other reasonable error mail get rate limited.

Diff Detail

Repository
rP Phabricator
Branch
errmail2
Lint
Lint Passed
Unit
Tests Passed

Event Timeline

epriestley retitled this revision from to When we fail to process mail, tell the user about it.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Apr 4 2014, 1:00 AM
epriestley updated this revision to Diff 20630.

Closed by commit rPd9cdbdb9fae9 (authored by @epriestley).