Bug 174872 - Dashboard: JavaScript exception when expected elements are not present in Buildbot data
Summary: Dashboard: JavaScript exception when expected elements are not present in Bui...
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-26 13:34 PDT by Aakash Jain
Modified: 2017-07-26 13:54 PDT (History)
3 users (show)

See Also:


Attachments
Proposed patch (2.42 KB, patch)
2017-07-26 13:42 PDT, Aakash Jain
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-07-26 13:34:50 PDT
Dashboard code can hit exceptions while trying to access the elements which are not present in the data. We should check if the elements are present before parsing the data from them. This condition is hit particularly while using buildbot 0.9.
Comment 1 Aakash Jain 2017-07-26 13:42:20 PDT
Created attachment 316471 [details]
Proposed patch
Comment 2 Daniel Bates 2017-07-26 13:52:36 PDT
Comment on attachment 316471 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=316471&action=review

Can we write a unit test for this change?

> Tools/ChangeLog:3
> +        Dashboard: Don't crash when expected elements are not present in buildbot data

"Crash" is not the correct terminology to use to refer to a JavaScript exception or error.

> Tools/ChangeLog:9
> +        (BuildbotIteration.prototype.failureLogURL): check if _firstFailedStep.logs is present before accessing

Please capitalize the first letter in this sentence.

> Tools/ChangeLog:11
> +        (BuildbotIteration.prototype._parseData): check if data.sourceStamps is present before accessing its length.

Ditto.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotIteration.js:247
> +        else if (data.sourceStamps)

Is it possible for the version of Buildbot we use in OpenSource or internally to not have both sourceStamp and sourceStamps defined. If not, then I suggest we defer making this change until we upgrade OpenSource to the latest version. I know that the latest Buildbot does not have this property from talking with you today.