fix: recover if protocol does not support CSS rule usage #1479
Conversation
There was a problem hiding this comment.
LGTM.
What was the deal with @brendankenny's recoverable error stuff? Would that be a good fit?
|
#1482 :) |
| @@ -25,7 +25,8 @@ class CSSUsage extends Gatherer { | |||
| beforePass(options) { | |||
| return options.driver.sendCommand('DOM.enable') | |||
| .then(_ => options.driver.sendCommand('CSS.enable')) | |||
| .then(_ => options.driver.sendCommand('CSS.startRuleUsageTracking')); | |||
| .then(_ => options.driver.sendCommand('CSS.startRuleUsageTracking')) | |||
| .catch(_ => this.failure = 'CSS Usage tracking requires Chrome \u2265 56'); | |||
There was a problem hiding this comment.
maybe add a TODO to remove this once m56 is released as stable?
There was a problem hiding this comment.
also this marks all errors from beforePass as this error. To be fair, the previous code was anticipating no errors :) but probably better to check that it's 'CSS.startRuleUsageTracking' wasn't found or whatever so that other possible errors aren't muted
|
recoverable errors will work, they're just a manual replacement for -1 right now since the last step of #941 isn't in place yet, so probably fine just to use -1 for now |
|
one interesting thing that this brings up that we didn't anticipate with a recoverable error is if |
|
Interesting what's wrong with |
|
i'm ok with finally as well :) Seems a good idea to clean up all the things! Is it an option for the future to change the output of the formatter? (if < chrome 56 show coming soon icon, ...) so it doesn't look like a failure. |
in theory nothing, but we'd really have to nail down when it runs, its relation to the artifact produced, etc. e.g. will it always make sense to clean up after |
R: @wardpeet all
fixes #1473