Usage of Router inside a DurableObject: need to move env, which is required to be a reference
Hi,
Apologies in advance if I'm missing something obvious here, I'm new to Rust. I'm trying to use Router in a #[durable_object]s fetch method like so:
use worker::*;
#[durable_object]
pub struct Chatroom {
messages: Vec<String>,
state: State,
env: Env,
}
#[durable_object]
impl DurableObject for Chatroom {
fn new(state: State, env: Env) -> Self {
Self {
messages: vec![],
state,
env,
}
}
async fn fetch(&mut self, req: Request) -> worker::Result<Response> {
let router = Router::with_data(&self.messages);
router
.get("/", |_req, context| {
Response::ok(&format!("{} messages", context.data().len()))
})
.run(req, self.env)
.await
}
}
This fails with the error:
error[E0507]: cannot move out of `self.env` which is behind a mutable reference
--> src/chatroom.rs:27:23
|
27 | .run(req, self.env)
| ^^^^^^^^ move occurs because `self.env` has type `worker::Env`, which does not implement the `Copy` trait
This makes sense to me, as Router#run takes ownership of it's env argument, and since I'm taking a &mut self as a reference I can't move self.env. The #[durable_object] macro forces fetch to be &mut self so I can only give references of env. I can't work out how you would use Router inside it?
Great point, and thanks for bringing this up. I'll set up a project to reproduce this asap and get back to you in the next couple of days.
It'd be great to have something like this working on. I'm happy to help if needed.
@CRogers, I did this as a workaround. Maybe it works for you as well.
async fn fetch(&mut self, req: Request) -> worker::Result<Response> {
let router = Router::with_data(&self.messages);
router
.get("/", |_req, context| {
Response::ok(&format!("{} messages", context.data().len()))
})
.run(req, self.env.clone().into())
.await
}
clone() returns a JsValue, into() converts that to an Env
@Maurimura7 - please do take a look and help if you'd like, all help is appreciated! I have another project at the moment taking priority, so this might not be something that I'll be able to dive into for another week or so.
Hey, @nilslice I tried to derive the Copy trait on the Env definition but without luck. So I decided to continue with the workaround until I get something working this way I could see the benefits from the current implementation, for this, I've created a wishlist-like project using Durable Objects as my persistent storage.
Everything went well at first, I was able to get data from storage pretty quick, but when I tried to put in some data problems started to appear.
I was passing self to the router like this
let router = Router::with_data(self);
But when I tried to do
router
.post_async("/item", |mut req, ctx| async move {
let mut _self = ctx.data();
let mut storage = _self.state.storage();
storage.put("items", "new item").await?;
Response::ok("Created")
this wasn't compiling because of the put method expects a &mut self and ctx.with_data() give us a &self. So, my next workaround was wrapping self like this:
let router = Router::with_data(Rc::new(RefCell::new(self)));
now I can borrow a mutable reference to self by calling ctx.data().borrow_mut(), and therefore, put things in the storage.
My fetch function looks like this
async fn fetch(&mut self, req: Request) -> Result<Response> {
let env = self.env.clone().into();
let router = Router::with_data(Rc::new(RefCell::new(self)));
router
.get("/items", |_req, ctx| {
let mut _self = ctx.data().borrow_mut();
Response::from_json(&json!({"items": _self.items}))
})
.get_async("/storage/items", |_req, ctx| async move {
let mut _self = ctx.data().borrow_mut();
let items = _self.state.storage().get("items").await?;
Response::from_json(&json!({ "items": items }))
})
.post_async("/items", |mut req, ctx| async move {
let mut _self = ctx.data().borrow_mut();
let mut storage = _self.state.storage();
if let Ok(body) = req.json::<Item>().await {
_self.items.push(body.payload);
storage.put("items", _self.items.clone()).await?;
Response::ok("Created")
} else {
Response::error("Bad Request", 400)
}
})
.run(req, env)
.await
}
What do you think about it? Do you see any downsides of doing this?
@Maurimura7 -
Thank you for sharing your solution. I think this is the maybe the only way to accomplish things without an API change within the Durable Object macro. As far as downsides, the clone calls shouldn't be that bad, but will incur a cost. It will be interesting to see how much you are impacted here though...
Additionally, (and this is the same for the Worker use of Router) it must be created and dropped each time the DO gets a request. This is less of an issue with the non-DO Worker, since there are many of them spawned and the impact of memory usage is spread across each invocation. But with the DO, which is always a single instance, the creation and re-creation of the Router could possibly become an issue depending on the memory usage of other tasks in the DO.
I would keep an eye on the number of concurrent requests to the DO and if you end up seeing memory errors causing the DO to fail, using the Router here may be the culprit.
Hey @nilslice thanks for your feedback, I really appreciate it. I'll keep an eye on it and see how the memory evolves.
One thought I had while working on it was if this router is really needed for Durable Objects. At least for my use case, I'm fine with having something that matches my URL and method, since I'm talking with it through another Worker, I get all the benefits from the router there. I did a silly experiment with it and the code become more straightforward and perhaps more beginner-friendly, because I didn't have to use any smart pointer to get it working. But this could be a bit difficult to scale if the use cases become more complex.
I'll keep playing with them and see if I can have more suggestions about it.
@Maurimura7 - totally agree, the Router is really only helpful when you are constructing more complex applications. Ideally as much logic is executed in the Worker layer vs. the DO, so I think it's 100% fine to omit it entirely from the DO implementation. But it's there if you need it ;)