Can Template::custom() take an FnOnce?

[anna-is-cute]

Rocket version: 0.4.2
Steps I've taken to answer the question myself: looked at the source and mentally examined the needs of such a closure.
Documentation I believe should include an answer: none.

Basically, see title. Is there a need for Template::custom() to take Fn instead of FnOnce?

I would like to initialise some values and do error handling outside of the closure, then move a value inside the closure and use it there. Unfortunately, this is not possible, since the type is not Copy nor Clone.

fn main() {
  // not copy or clone
  let something = do_some_initialisation();

  rocket::ignite()
    .attach(Template::custom(move |engines| {
      // move `something` in here
    })
    .launch();
}

This is currently impossible due to the constraints of Fn.

If Template::custom() can take an FnOnce, I'd be happy to PR such a change. :)

[jebrosen]

Template::custom was introduced in #521. At the time Fn was only used instead of FnOnce because of #522. But with #537 the function will actually be called multiple times in debug mode in the case that templates have been changed on disk.

What is the nature of something that it can only be used once?

EDIT: Hit the button too fast.

[anna-is-cute]

In my specific case, it's a list of localisation bundles. I would like to handle loading them and then handle any errors outside of the closure, then move the vector inside the closure when appropriate.

That vector gets moved to another function, which creates a Tera function for producing localised messages, referencing that vector for all queries.

The localisation bundles aren't Copy or Clone, so I can't clone the vector, unfortunately. Right now I'm just unwrapping inside the closure to make sure the server doesn't start without localisation.

[jebrosen]

That vector gets moved to another function, which creates a Tera function
The localisation bundles aren't Copy or Clone, so I can't clone the vector, unfortunately.

That sounds... messy. I would suggest sprinkling a few Arcs around, but that could become pretty unreadable. Might be worth trying, at least.

If loading the bundle is relatively cheap or you're willing to sacrifice that time for each template reload in debug mode, you might load the bundle in the custom() closure instead. But then I suppose you can't handle errors beforehand, unless you loaded them both beforehand and in custom().

[anna-is-cute]

It's really not messy.

fn main() {
  let bundles = init_bundles();
  rocket::ignite()
    .attach(Template::custom(move |engines| {
      engines.tera.register_function("tr", create_tera_function(bundles));
    })
    .launch();
}

fn create_tera_function(bundles: Bundles) -> Box<tera::GlobalFn> {
  Box::new(move |args| {
    // ...
  })
}

That's essentially how it is. I don't think that's messy. However, that code is impossible because of Fn.

Loading the bundles is cheap enough to do in the custom() closure; that's what I'm doing right now. The problem is that I have no way to gracefully shut down the server if the bundles can't be loaded. It's nice that they reload when the templates do (I can change a template after adding localisation to easily test it), but there's no way for me to handle errors cleanly.

[jebrosen]

but there's no way for me to handle errors cleanly.

I think that might actually be the key issue here. Would this issue also be addressed if Template::custom could return an error which would be propagated up as an attach or launch error, if it happened the first time the templates were loaded?

The only real alternative I can think of on Rocket's side is to sacrifice template reloading.

[anna-is-cute]

Yeah, I agree it's a tough spot for Rocket.

Could it be possible to propagate the error every time the templates reload? Or do you mean it would be like when the templates fail to reload currently, where it just uses the last successfully-loaded templates and displays an error. That would be fine.

Template::custom returning an error works for me, though.

[jebrosen]

If we made this change, I imagine we would do the following:

1. Change dyn Fn(&mut Engines) + Send + Sync + 'static to dyn Fn(&mut Engines) -> Result<(), ???> + Send + Sync + 'static where ??? is some error type
2. In on_attach (https://github.com/SergioBenitez/Rocket/blob/master/contrib/lib/src/templates/fairing.rs#L158), log an error message and return Err(rocket) if the callback returns an error.
3. In reload_if_needed (https://github.com/SergioBenitez/Rocket/blob/master/contrib/lib/src/templates/fairing.rs#L104), log an error message if the callback returns an error. There's nowhere else for the error to go besides panicking because this happens in on_request.

[anna-is-cute]

There's nowhere else for the error to go besides panicking because this happens in on_request.

Yeah, so it would be similar to these lines.

If so, that all sounds good to me and gets the desired behaviour I was looking for. :)

[SergioBenitez]

@jebrosen Will you be taking a second look at this?

[SergioBenitez]

Whoops! Didn't mean to close.