search

catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.91k stars 563 forks source link

run_metric sometimes silently fails #4494

Open tdresser opened 6 years ago

tdresser commented 6 years ago

It looks like there are cases where run_metric will silently die.

I think it's the best way to iterate on a new metric definition, so silently dying is unfortunate.

tdresser commented 6 years ago

@deepanjanroy

benshayden commented 6 years ago

Can you provide more detail? What OS[s] does this happen on? Which metrics? Does it happen reliably for any particular traces?

tdresser commented 6 years ago

Sorry, that was a terrible bug report.

This is when I write code which is legitimately broken, during metric computation. Instead of dumping the error to stdout, run_metric just silently dies.

This specific example was withing constructLoadingExpectation_.

benshayden commented 6 years ago

There's a try-catch in UserModelBuilder that converts exceptions to model.importWarnings so that old traces can be imported. Maybe map_single_trace_cmdline.html should print importWarnings after importing the trace? @eakuefner

eakuefner commented 6 years ago

Ben and I talked about this offline -- definitely printing import warnings is a good idea and it'll probably require this information to be plumbed through MreResult. Happy to help review if someone wants to take a look at this.