fix: use lazy argument evaluation to handle errors #9053
Conversation
| let rss_config = rss_config_path.exists().then(|| { | ||
| let rss_config = | ||
| RackInitializeRequest::from_file(rss_config_path) | ||
| .map_err(|e| CmdError::Failure(anyhow!(e)))?; |
There was a problem hiding this comment.
I'm not sure this compiles - does this mean rss_config is now an Option<Result<...>>?
There was a problem hiding this comment.
Ugh. Of course. :-(
There was a problem hiding this comment.
Actually, it didn't mean that; then_some was already returning an Option<RackInitializeRequestParams>; the Result was being destructured by the ? on line 68. But you can't (conveniently) use ? in a closure, so it did fail to compile.
It's possible one could omit the ? and try to play games with .transpose or .ok or some combination of things like that, but I don't think the result would be comprehensible. My conclusion is that none of the slick methods on bool are really useful here, so I just switched it back to an if expression.
An instance of `.then_some` on a bool caused an error because `.then_some` takes a 'T' as an argument, and is thus evaluated strictly; an error in the resulting eager evaluation caused an error return, regardless of whether the predicating boolean was true or not. Nominally, using `.then` would have fixed it, but one cannot conveniently use `?` in a closure, which the code we wanted to run did. So switch to a good 'ol `if` expression instead. In testing, all of our configurations had the file we were testing the presence of before reading. But other folks do not. However, one cannot conveniently use `?` in a closure in this context, so Fixes #9051
An instance of `.then_some` on a bool should have been `.then`. The former evaluates its arguments eagerly, so runs _regardless of the value of the bool_, while the latter does so lazily, which was the desired behavior. In testing, all of our configurations had the file we were testing the presence of before reading. But other folks do not. Fixes #9051
An instance of
.then_someon a bool should have been.then. The former evaluates its arguments eagerly, so runs regardless of the value of the bool, while the latter does so lazily, which was the desired behavior.In testing, all of our configurations had the file we were testing the presence of before reading. But other folks do not.
Fixes #9051