treeherder-ui should handle suggestion.bugs being null more gracefully

RESOLVED FIXED

Status

P2
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: emorley, Assigned: emorley)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
The fix for bug 1059306 doesn't apply to previously parsed logs - to help with those we could just handle this more gracefully in the UI:

"Error: suggestion.bugs is null
update_bugs/</<@https://treeherder.allizom.org/ui/js/index.min.js:19:18184
forEach@https://treeherder.allizom.org/ui/js/index.min.js:4:31160
update_bugs/<@https://treeherder.allizom.org/ui/js/index.min.js:19:18133
qFactory/defer/deferred.promise.then/wrappedCallback@https://treeherder.allizom.org/ui/js/index.min.js:7:26466
qFactory/ref/<.then/<@https://treeherder.allizom.org/ui/js/index.min.js:7:27876
$RootScopeProvider/this.$get</Scope.prototype.$eval@https://treeherder.allizom.org/ui/js/index.min.js:8:4393
$RootScopeProvider/this.$get</Scope.prototype.$digest@https://treeherder.allizom.org/ui/js/index.min.js:8:2247
$RootScopeProvider/this.$get</Scope.prototype.$apply@https://treeherder.allizom.org/ui/js/index.min.js:8:4818
done@https://treeherder.allizom.org/ui/js/index.min.js:7:2339
completeRequest@https://treeherder.allizom.org/ui/js/index.min.js:7:6224
createHttpBackend/</xhr.onreadystatechange@https://treeherder.allizom.org/ui/js/index.min.js:7:7168
"

https://github.com/mozilla/treeherder-ui/blob/c1abd6237b4d00684b1c410728309e3f12a2c181/webapp/app/plugins/failure_summary/controller.js#L41
(Assignee)

Comment 1

4 years ago
Created attachment 8480787 [details] [diff] [review]
Patch v1

I'm not sure what to do with this - either we use something hacky like this patch and remove it at a later date (given the service is now fixed), or we add more complete checking for missing properties. (But for the latter, we don't do this for everything else, and seems like we're at risk of bloating the UI code if we do it across the board).
Attachment #8480787 - Flags: feedback?(mdoglio)
(Assignee)

Updated

4 years ago
Attachment #8480787 - Attachment is patch: true
Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Comment 4

4 years ago
Jonathan/Cameron, would either of you mind taking the review for this? (Didn't realise Mauro was away)
Comment on attachment 8480787 [details] [diff] [review]
Patch v1

Your solution is fine for now. I agree with your observations in comment 1 regarding property checking. I think the way to go there would be to have json schema validation that's specific to different job artifacts. This should be done with one set of javascript classes and implement the validation in a single place in the application. Out of the scope of this bug, would be a good task for a refactor in the future.

It will be more of an issue as we receive more types of data other than buildbot job artifacts, ones from jenkins/taskcluster will likely have different requirements for validation. So this could get messy fast without some better validation strategy.
Attachment #8480787 - Flags: review+
(Assignee)

Comment 6

4 years ago
Comment on attachment 8480787 [details] [diff] [review]
Patch v1

https://github.com/mozilla/treeherder-ui/commit/0bdb50c7f6a49da7ea680ea85fd0ac546f2fae99

Thank you - I'll file another bug for implementing more thorough json
schema validation in the ui.
Attachment #8480787 - Flags: checkin+
(Assignee)

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

4 years ago
(In reply to Ed Morley [:edmorley] from comment #6)
> Thank you - I'll file another bug for implementing more thorough json
> schema validation in the ui.

Actually bug 1043338 is already filed for this.
(Assignee)

Updated

4 years ago
Attachment #8480787 - Attachment description: WIP → Patch v1
Attachment #8480787 - Flags: feedback?(mdoglio)

Comment 8

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

https://github.com/mozilla/treeherder/commit/a222d4575502e790c07c7dcb79f52d4cd6fe3c70
Bug 1059949 - Handle bugs not having open_recent and all_other keys more gracefully; r=mdoglio

For a short time, treeherder-service was creating log artefacts that
were missing the open_recent and all_other keys on the bugs
property. This likely temporary workaround will let us view
the summaries for those jobs until enough time has passed
that we no longer care about them.
You need to log in before you can comment on or make changes to this bug.