Page MenuHomePhabricator

Phabricator sometimes sends mail that it shouldn't
Closed, DuplicatePublic

Description

This is a bit of a strange twist in the spirit of "Phabricator sends too much mail" and I'm afraid it will be lacking reliable reproduction steps, as I can't figure any out yet. However, I think it may be worth starting the conversation in case others have experienced this, and perhaps through conversation and further investigation we as a community can figure out how to reproduce it.

The issue is this: Sometimes Phabricator seems to send mail when it shouldn't.

  1. We prepared user accounts for some of our users and began using them, e.g. assigning tasks to them, before they validated their email addresses, yet Phabricator occasionally sent them mail. Mostly it didn't, but sometimes it did. I think the most commonly sent mails were "Block/Unblock" notifications, but don't think they were the only ones.
  2. Sometimes Phabricator sends emails despite your email preferences. For example, I have opted not to receive emails about column changes, yet sometimes I receive emails notifying me about column changes (and only column changes).

It seems very strange, and I can't fathom what could be causing it, nor do I have enough experience with Phabricator yet to really know how to investigate it further.

It's been happening for at least about a year, I'd say. Currently my instance is running these versions which aren't cutting edge, but aren't all that old either:

phabricator 27006fedccc2f487dcaf7177f6d127ccd0e6df7d (Sep 24 2016) 
arcanist e7906e47cba2006fa46553d462abf1f685845f20 (Sep 24 2016) 
phutil 518dacd785fb19d67643e4a113e99b8825a87f99 (Sep 24 2016)

Event Timeline

isfs created this task.Nov 9 2016, 10:37 AM
chad added a subscriber: chad.Nov 9 2016, 4:44 PM

/mail/ will tell you why you received any given mail.

chad closed this task as Resolved.Nov 9 2016, 4:48 PM
chad claimed this task.

Closing as Resolved since this doesn't seem actionable by the upstream without concrete reproducible steps.

isfs changed the task status from Resolved to Invalid.Nov 10 2016, 9:20 PM

I wouldn't call this resolved. Invalid may be most relevant, as with no STR, we can't confirm or action this.

I looked at /mail/ for scenario 2, and mails that are being sent have a <manifest-other> tag. I'm not sure why. Looking at the timeline, I see this:

  • The column movement email on Nov 8 was correctly voided because it was only tagged <manifest-column>.
  • The column movement email on Nov 9 at 3:09 PM was incorrectly sent; it was tagged <manifest-column> and <manifest-other>
  • The reassignment from shashanka to justin actually took place at 3:11 PM; the email about it was correctly sent (tagged <manifest-owner>).

When I get some time I might try digging around the source and/or database to see if I can figure out what's going on. Any pointers would be welcome.

The mail logs don't go far enough back for me to investigate scenario 1, but I may try to dig up a backup and install on a virtual machine to see if I can find out anything and/or create a new dummy user and subscribe it to some tasks and see if it receives email.

Hopefully after some more investigation I can reproduce it.

isfs updated the task description. (Show Details)Nov 10 2016, 9:21 PM
chad added a comment.Nov 10 2016, 9:39 PM

Do you have custom fields or custom forms?

isfs added a comment.Nov 10 2016, 9:50 PM

Yes, both. I don't see how either of those could be involved with a column change, though, could it? Changes to our custom field do show on the timeline, too, but nothing is showing here.

chad added a comment.Nov 10 2016, 9:52 PM

There might be some crazy edge case of empty custom field that was previously hidden in a custom form and is being initialized for the first time, but not generating a visible transaction, so is getting lumped under "other". Hard to say, I can't really recreate how this might happen. Do you have "other" email sent to "on" but everything else "off" ? Knowing those would be important in a reproduction case as well.

isfs added a comment.Nov 10 2016, 10:06 PM

Initialisation doesn't seem to fit the pattern, as I would have expected an earlier column change to trigger it if that would. There are also over a dozen transactions on the timeline of this task before the section I pasted here, including comments (one deleted; I wonder if that's relevant), column changes, reassignments, and status changes.

And yes, I do have "other" email turned on, so it makes perfect sense for me to receive notifications about "other" transactions. So I guess part of the bug is identified: "other" transactions are tagged and thus the email sent even if the transaction isn't visible.

But to be able to reproduce it, we kinda need to know what the "other" transaction actually was, don't we?

isfs added a comment.Nov 10 2016, 10:29 PM

OK. The type of the other transaction is subpriority. From the database:

             id: 9641
           phid: PHID-XACT-TASK-th2fmuxardoypm2
     authorPHID: PHID-USER-55zwskc2gpy2olvlwnjf
     objectPHID: PHID-TASK-fx32td737asu3gpgyi3c
     viewPolicy: public
     editPolicy: PHID-USER-55zwskc2gpy2olvlwnjf
    commentPHID: NULL
 commentVersion: 0
transactionType: subpriority
       oldValue: "0.0077892676007758"
       newValue: 0.0057832296190288
  contentSource: {"source":"web","params":[]}
       metadata: []
    dateCreated: 1478664574
   dateModified: 1478664574

This transaction shares the same creation and modification date as the column change. I guess the 'subpriority' is used to maintain the task order on the workboard, so this probably means the task was dragged from one column to another (rather than using the 'Move to Column' action). It may only be relevant if the workboard is set to order by priority, rather than natural. This fits with my experience, as most incorrectly sent mail is about column changes, and I have seen an increase in such mail correlating with an increase in dragging tasks on our workboards.

I'll see if I can reproduce it on a clean install.

chad added a comment.Nov 10 2016, 10:31 PM

Hmm yeah, we record where in the workboard card is in the list, sticky. We could probably ignore that transaction.

chad reopened this task as Open.Nov 10 2016, 10:34 PM
isfs added a comment.Nov 10 2016, 10:41 PM

Related thought: I wonder how the sticky task order fares when a task is on multiple workboards; since the subpriority seems to be a task parameter, not a workboard card parameter, I guess moving it on one workboard may clobber its position on another; quite possibly not a problem in practice.

isfs added a comment.Nov 10 2016, 11:31 PM

Re scenario 1, I found another way to investigate it rather than digging up old logs. Question, though: is Phab supposed to send email to users who are unverified? Looking at mail logs, it seems one of our most recently added users was sent quite a lot of mail, perhaps all mail they would ordinarily get, prior to verification. If Phab is supposed to send mail to unverified users, then this would be an enhancement request which I could submit with a proper description of the use case. If not, I can investigate how to reproduce the behaviour.

is Phab supposed to send email to users who are unverified?

Yes.

Disabled users do not receive mail, but unverified users (and unapproved users) do.

https://secure.phabricator.com/diffusion/P/browse/master/src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php;ff677c1964b15d61cf9a4ddd727c1da97b63fe00$51-94

At a minimum, we need to send unverified users verification email.

isfs added a comment.Nov 11 2016, 12:59 AM

Thanks for that confirmation. I like the look of your code comment: "Possibly we should white-list this kind of mail and deny all other types of mail." I'll create an enhancement request task related to that.

We should restrict this bug report to addressing scenario 2.

isfs updated the task description. (Show Details)Nov 11 2016, 12:59 AM

Dragging a card on a workboard while in a priority-ordered view changes the global priority (see T8135). Mail tags in "other" groups are arbitrary, unclear, and going away (see T10448). I'm going to merge this into T10448.