jonhoo / fantoccini

A high-level API for programmatically interacting with web pages through WebDriver.
Apache License 2.0
1.68k stars 125 forks source link

feat: Improved error handling by using `thiserror` #208

Open OtaK opened 1 year ago

OtaK commented 1 year ago

No idea if you would like this to be in; Seems to simplify things a lot when it comes to implementing & extending errors.

My branch is (very) out of date compared to the main branch. I'll get to rebasing it soon.

jonhoo commented 1 year ago

Thanks! I'm not opposed to the idea, though I'm curious how much this adds to build time — could you measure that (cargo build --timings) and report?

OtaK commented 1 year ago

Sure - I made sure to run cargo clean between executions. Seems there's a small compilation time gain (probably because the error handling impl is not inline compared to a handwritten impl)

Also rebased this branch on top of main so it should be up to date

Main branch

Cargo Build Timings — fantoccini 0.20.0-rc.4-main.pdf

This branch

Cargo Build Timings — fantoccini 0.20.0-rc.4-feat-error-handling.pdf

jonhoo commented 1 year ago

Huh, that's surprising — usually extra proc macros add a decent amount to build time even just because thiserror would have to be built first. It could be because you're building with 32 cores, and so the added size of the dependency graph doesn't matter too much, though I think I'm fine merging a change like this anyway. Thanks for measuring :+1: