dmi3kno / polite

Be nice on the web
https://dmi3kno.github.io/polite/
Other
327 stars 14 forks source link

politely should use on.exit to restore HTTPUserAgent #29

Open DarwinAwardWinner opened 4 years ago

DarwinAwardWinner commented 4 years ago

It looks like politely calls options to set the user agent prior to running the wrapped function and then calls options again afterward to restore the previous value. However, if the wrapped function throws an error, the second call to options never happens and the user agent is not restored. I believe the correct way to handle this is with on.exit, e.g.:

old_ua <- getOption("HTTPUserAgent")
on.exit(options("HTTPUserAgent"= old_ua))
options("HTTPUserAgent"= user_agent)
res <- mem_fun(...)
dmi3kno commented 4 years ago

Thanks for the tip. Will fix ASAP

dmi3kno commented 2 years ago

I am closing the issue, as the feature has been merged to the main repo. Feel free to open another one if you notice some other strange behavior.

DarwinAwardWinner commented 2 years ago

One minor note: it's generally good practice (as I've learned in the time since reporting this issue) to call on.exit with add = TRUE. This ensures that if you ever add a 2nd on.exit in the same function for unrelated reasons, both expressions will get run. Otherwise the 2nd one will override the first.

DarwinAwardWinner commented 1 year ago

Btw, I think this issue can be closed now. It looks like the PR merge didn't auto-close it.