Bug 193563 - Analyzing a chart that does not exist should not halt whole run-analysis script.
Summary: Analyzing a chart that does not exist should not halt whole run-analysis script.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: dewei_zhu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-17 22:00 PST by dewei_zhu
Modified: 2019-01-22 20:30 PST (History)
2 users (show)

See Also:


Attachments
Patch (5.07 KB, patch)
2019-01-17 22:05 PST, dewei_zhu
no flags Details | Formatted Diff | Diff
Patch (8.38 KB, patch)
2019-01-19 00:58 PST, dewei_zhu
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description dewei_zhu 2019-01-17 22:00:38 PST
Analyzing a chart that does not exist should not halt whole run-analysis script.
Comment 1 dewei_zhu 2019-01-17 22:05:29 PST
Created attachment 359449 [details]
Patch
Comment 2 Ryosuke Niwa 2019-01-18 20:31:02 PST
Comment on attachment 359449 [details]
Patch

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

> Websites/perf.webkit.org/ChangeLog:17
> +        (async.set requests):
> +        (async):

Remove these?

> Websites/perf.webkit.org/tools/js/measurement-set-analyzer.js:55
> +        } catch (error) {
> +            this._logger.warn(`Skipping analysis for "${metric.fullName()}" on "${platform.name()}" as time series does not exit.`);

Please check/assert for a specific error instead of swallowing any exception.
Comment 3 dewei_zhu 2019-01-19 00:58:26 PST
Created attachment 359598 [details]
Patch
Comment 4 Ryosuke Niwa 2019-01-22 19:24:25 PST
Comment on attachment 359598 [details]
Patch

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

> Websites/perf.webkit.org/public/v3/models/measurement-set.js:86
> +        if (promise && callback) {
>              promise.then(callback, callback);
> -        else {
> +        } else if (!promise) {

It would be cleaner to nest if's as in:
if (promise) {
    if (callback)
        ~
} else {
    ~
}
Comment 5 dewei_zhu 2019-01-22 20:30:13 PST
Landed in r240319.