On Thu Feb 26, 2026 at 10:28 AM CET, Dominik Rusovac wrote: > I agree, unwrapping the option is indeed questionable. Yet these unwraps > seemed good enough and I didn't want to touch the error handling itself, > as it would entail more changes. Atm, we have no designated Error types > to map to and imo just bail!-ing on a Result in this place adds no > reasonable value. Consistently propagating errors from start to end > would be more appropriate, I suppose. > > For now, I just added some context as to what must have gone wrong in > this very situation, because it changes just a tiny bit of code, while > adding reasonable information. > > I could send a v2 that touches error handling in general, though.
Currently, the only user of score_alternatives(...) is score_nodes_to_start_service(...), where it's enough that nodes is an empty slice to cause a panic. This isn't a problem now because the function is unlikely to be called as there's always at least one node available. If we're going to use these for the load balancer, it could be way easier to run into this, e.g. there is only one service restricted to a single node, i.e., no migration candidates. Of course we could catch this early, but returning a generic Error here and then handling that in score_alternatives(...) to return an empty vec would be an improvement. Having error types would be plus here of course, but needs some consideration that it doesn't break building packages that use this crate.
