1513485 - Cannot manually create alerts

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect, P1)

Description

[Ionuț Goldan [:igoldan]]

Attachment: forever-waiting-on-creating-new-alert.png

When trying to manually create alerts from Graphs view, the "Creating alert..." spinner is running forever and no alert is generated.

I'm pretty sure this worked fine until the last deploy.

Comment 1

[Ed Morley [:emorley]]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #0)
> I'm pretty sure this worked fine until the last deploy.

The last deploy was for https://github.com/mozilla/treeherder/compare/8ac1515494a8...6c3f8982873c , which includes both bug 1506671 and bug 1506553.

Comment 2

[Ionuț Goldan [:igoldan]]

(In reply to Ed Morley [:emorley] from comment #1)
> (In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #0)
> > I'm pretty sure this worked fine until the last deploy.
> 
> The last deploy was for
> https://github.com/mozilla/treeherder/compare/8ac1515494a8...6c3f8982873c ,
> which includes both bug 1506671 and bug 1506553.

I started debugging this. So far, it looks like bug 1506553 caused this issue.
The createAlert() function from treeherder/ui/perfherder/helpers.js incorrectly used the create() function treeherder/ui/helpers/https.js

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4367

Comment 4

[Ionuț Goldan [:igoldan]]

Comment on attachment 9030751 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4367

Review of attachment 9030751 [details] [review]:
-----------------------------------------------------------------

I made a push which re enables the create alert functionality.
Bad thing now is the console dumps some errors I'm not yet sure how to follow.
From the looks of it, Angular watches a function call expression. One of its parameters
is somehow mutated during a digest cycle, causing an attribute access error.

In case you're used with such peculiarities, please share your thoughts. Otherwise, I'll just resume the investigation tomorrow.

Comment 5

[Ed Morley [:emorley]]

To clarify, the dependency tree change is since this regression bug was caused by bug 1506553 (comment 2), and we typically mark the regression bug as blocking the one that caused it. See bug 1513483 comment 4 for more details on this workflow :-)

Comment 6

[Sarah Clements [:sclements]]

Comment on attachment 9030751 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4367

:igoldan, can you give me the STR since I'm not that familiar with this part of the App? Does it require sheriff privileges to trigger an alert (if so, is there a way to work around that requirement for development purposes)? If you could paste the console message that would be helpful. One thing I noticed by looking at the `CreateAlert` method is that it's not catching errors so adding that and console.logging within that catch method and those nested `.then()`s might give more insight.

Comment 7

[Ionuț Goldan [:igoldan]]

The STR are these: simply go to any graph, select a data point and from the black popup that shows up, click the "create" hyperlink.

Comment 8

[Ionuț Goldan [:igoldan]]

I'm not sure about the privileges. I have to look that up in the source code.

Comment 9

[Sarah Clements [:sclements]]

Ok, so I see "No alert (log in as a a sheriff to create)" rather than "create hyperlink" on the tooltip. If you still need help debugging, please post a screen shot with the errors or paste them here.

Comment 10

[Ed Morley [:emorley]]

(In reply to Sarah Clements [:sclements] from comment #6)
> Does it require sheriff privileges to trigger an alert (if so,
> is there a way to work around that requirement for development purposes)?

I've set `is_staff` to 1 on stage+prod for your account (couldn't do prototype, since the user doesn't exist yet) - thought you had access already, but perhaps that was for your gmail? :-)

Comment 11

[Sarah Clements [:sclements]]

I do seem to have access now when logging in - thanks Ed!

Comment 12

[Ionuț Goldan [:igoldan]]

Attachment: errors-when-manually-create-alerts.png

Comment 13

[Ionuț Goldan [:igoldan]]

(In reply to Sarah Clements [:sclements] from comment #9)
> Ok, so I see "No alert (log in as a a sheriff to create)" rather than
> "create hyperlink" on the tooltip. If you still need help debugging, please
> post a screen shot with the errors or paste them here.

Yes, I could use your help on this one. I've attached a screenshot with the errors.

Comment 14

[Sarah Clements [:sclements]]

I'm not seeing those errors when testing out your local commit and creating an alert (against stage). It seems to be generating Alerts properly. I did leave some comments on your pull request.

Comment 15

[Ionuț Goldan [:igoldan]]

(In reply to Sarah Clements [:sclements] - away until 12/26 from comment #14)
> I'm not seeing those errors when testing out your local commit and creating
> an alert (against stage). It seems to be generating Alerts properly. I did
> leave some comments on your pull request.

Yes, I managed the fix the errors by tweaking the graphsctrl.html file.

Comment 16

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/bc3b45fac93625f3252a02c76802b437832ea544
Bug 1513485 - Properly fetch response (#4367)