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
Unknown Object (File)
Wed, Apr 24, 10:49 PM
Unknown Object (File)
Thu, Apr 11, 10:03 AM
Unknown Object (File)
Mon, Apr 8, 11:42 PM
Unknown Object (File)
Mon, Apr 8, 5:34 AM
Unknown Object (File)
Mon, Apr 8, 2:50 AM
Unknown Object (File)
Feb 11 2024, 1:15 PM
Unknown Object (File)
Feb 4 2024, 7:20 PM
Unknown Object (File)
Feb 4 2024, 7:19 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
Lint
Lint Skipped
Unit
Tests Skipped

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).