search

paritytech / bench-bot

ISC License
9 stars 19 forks source link

Improve logging and error handling #46

Closed joao-paulo-parity closed 3 years ago

joao-paulo-parity commented 3 years ago

Note: "Draft" while I wait for some ongoing benchmarks to finish before redeploying the bot

Problem

The bot was spotted failing silently recently e.g. in

https://github.com/paritytech/substrate/pull/9018

where it would start the command but never give an actual response of what happened after.

Looking at the logs I see some unknown failures:

``` INFO (http): POST / 500 - 1541ms err: { "type": "Error", "message": "failed with status code 500", "stack": Error: failed with status code 500 at ServerResponse.onResFinished (/home/benchbot/bench-bot/node_modules/pino-http/logger.js:73:38) at ServerResponse.emit (events.js:327:22) at ServerResponse.EventEmitter.emit (domain.js:483:12) at onFinish (_http_outgoing.js:723:10) at onCorkedFinish (_stream_writable.js:673:5) at afterWrite (_stream_writable.js:490:5) at afterWriteTick (_stream_writable.js:477:10) at processTicksAndRejections (internal/process/task_queues.js:83:21) } ERROR (server): Internal Server Error Error: Internal Server Error at Request.callback (/home/benchbot/bench-bot/node_modules/superagent/lib/node/index.js:883:15) at IncomingMessage. (/home/benchbot/bench-bot/node_modules/superagent/lib/node/index.js:1126:20) at IncomingMessage.emit (events.js:327:22) at IncomingMessage.EventEmitter.emit (domain.js:483:12) at endReadableNT (_stream_readable.js:1220:12) at processTicksAndRejections (internal/process/task_queues.js:84:21) ```

Also sometimes shelljs fails

``` ERROR (ShellJSInternalError): ENOENT: no such file or directory, open '/tmp/shelljs_afe5946f1b74fc61c3cd' ShellJSInternalError: ENOENT: no such file or directory, open '/tmp/shelljs_afe5946f1b74fc61c3cd' at Object.openSync (fs.js:476:3) at Object.readFileSync (fs.js:377:35) at execSync (/home/user/w/bench-bot/node_modules/shelljs/src/exec.js:89:17) at Object._exec (/home/user/w/bench-bot/node_modules/shelljs/src/exec.js:205:12) at Object.exec (/home/user/w/bench-bot/node_modules/shelljs/src/common.js:335:23) at BenchContext.self.runTask (/home/user/w/bench-bot/bench.js:24:48) at prepareBranch (/home/user/w/bench-bot/bench.js:87:16) at async /home/user/w/bench-bot/bench.js:359:23 ```

And also a bug introduced by #42

``` ERROR (event): report.substring is not a function TypeError: report.substring is not a function at /home/benchbot/bench-bot/index.js:97:21 at async Promise.all (index 0) ```

Solution

In this PR we aim to improve error reporting and logging throughout the code while also trying to make it more concise.

Regarding the above library errors, we'll try to counteract by upgrading all third-party dependencies and favoring native Node.js libraries over them where applicable.

joao-paulo-parity commented 3 years ago

We have evidence of it working on Substrate: https://github.com/paritytech/substrate/pull/9286#issuecomment-875273119

I've also demoed the branch benchmarks in https://github.com/joao-paulo-parity/substrate/pull/1#issuecomment-875269906

This much is reasonable at the moment for the lack of a proper test suite or static code analysis. Merging.