Support for Rocket 0.5

[dbanty]

I know that Rocket 0.5 is still in development but I was wondering if you had any thoughts about how to support it? It brings with it async support via Tokio 1.0 which (as far as I know) will be incompatible with the current lambda_runtime crate. In other projects I've been using netlify_lambda which is a fork of the official project that's been updated for Tokio 1.0, so maybe that's a good option?

I don't know what else is changing with Rocket 0.5 that might disrupt things, but I'm happy to take a swing at making it work. I mostly just wanted to check in to see if you have plans to / a strategy for supporting it.

[servonlewis]

That would be awesome

[davidmaceachern]

I would be very interested in giving a helping hand on this one.

[dbanty]

I started working on this in a fork here. So far this:

1. Switches Rocket to a git dependency tracking "master"
2. Switches to lamedh_* from lambda_* which is the Netlify fork that supports Tokio 1.x
3. Starts a bunch of refactoring to support those first two

Right now I'm stuck on a lifetime issue. The Handler trait has a function called call which takes in a &mut self and must output a Future which is 'static. The code to actually handle the request produces a Future which is '_ tied to self. I don't know how to solve this without cloning, so I'm hoping someone more experienced than me can light the way 😅

[dbanty]

Well I sort of fixed it by adding a bunch of Arcs, but I had to remove some functionality around dynamically resetting the base path of the mounted routes for each request. I'm hoping there's a better way to achieve that anyway but I'll need to gather more information first.

Next step is trying to get the tests to pass. Currently none of them compile because:

1. They need to be async now to call the handler
2. Some test cases are in macros which don't seem to be constructing things correctly? Need to dig in more to find out.

[davidmaceachern]

Nice work Dylan, I saw Arc is used inside Rocket itself and in other projects that are using Tokio so that might be the correct approach.

I have addressed the async tests issues here.

The tests appear to compile now, but there is one remaining error.

error[E0603]: enum `Body` is private
  --> tests/rocket_tests.rs:6:19
   |
6  | use lamedh_http::{Body, Handler, Request, Response};
   |                   ^^^^ private enum

[dbanty]

Thanks David! I switched to Rust stable in my fork since that's supported on Rocket master and because the latest nightly was causing some build errors. I then managed to fix all the compilation issues 🎉! Tests can build and run now, though 7 are failing.

I think the next step is to see if there are any failing that aren't related to the feature I removed and fix those. Then we can see about re-implementing the base-path feature with the new code. Maybe we can strip the base path out of the request at call time instead of changing the registration of the paths? Not totally sure.

@GREsau would you be able to give us some insight to the purpose of BasePathBehaviour::RemountAndInclude? I get what it's doing, just not sure why the behavior exists from a user-perspective. My guess from the doc comments is that this is so the response headers will match the full path if you're using some sort of prefix or stage in API Gateway. If that's the only purpose we might be able to patch that somehow during the request? Though it'd be slower than remounting once.

If we do need to go the re-mount route we might have to use some internal mutability to make it work. Some sort of Cell maybe?

[dbanty]

Another update, all the unit tests now pass!! I did change all the path tests to not use a macro anymore for my own sanity with debugging, but I believe they're all effectively the same.

The implementation to get the RemountAndInclude is a bit messy and involves a mutex which will slow things down a bit. If anyone has ideas on improving that behavior it's definitely worth taking a look at. We might consider putting that option behind a feature flag so you can opt out and not need the overhead.

There is also now an API Gateway V2 request format which the lamedh crates support. I believe I've fished out the correct pieces needed in order to make stage/base path replacement work correctly though I'd love a second set of eyes on that as well.

I think now that all the tests pass the next step is to actually attempt to use this thing and fix any issues that come up. If all goes well this will be my last update on this issue for a while, I'm not planning on making a PR back to this upstream repo until Rocket 0.5 is actually released.

[GREsau]

@GREsau would you be able to give us some insight to the purpose of BasePathBehaviour::RemountAndInclude? I get what it's doing, just not sure why the behavior exists from a user-perspective. My guess from the doc comments is that this is so the response headers will match the full path if you're using some sort of prefix or stage in API Gateway.

I actually started writing a blog post on this last year but never got around to finishing it unfortunately - but your guess pretty much nails it.

You can configure your API Gateway to use:

• the default (execute-api) API Gateway URL, or a custom domain
• a constant API gateway stage, or specify the stage in the request path
• a constant base path or not (only applicable to custom domains)

The intention of RemountAndInclude is that you can deploy rocket-lamb using any combination of the above configurations, and it will "do the right thing" without you having to tell rocket-lamb what base-path/stage etc it should expect.

Otherwise, you would need your application to know what base path it will be running under and ensure you mount your routes at that base path (this is the behaviour with BasePathBehaviour::Include), or just completely ignore the base path in which case any absolute URLs generated by your application may be invalid from the client's point of view (this is the behaviour with BasePathBehaviour::Exclude).

I would much prefer it if there were a way to tell Rocket that part of the request's URL is a "base path" which it should ignore for routing purposes, similar to HttpRequest.PathBase in ASP.NET Core

Also, determining the base path from the API Gateway request object is a bit of a hassle (as you can probably guess from the mess in RequestExt). If you're curious, check this gist for a list of possible API gateway URL configurations, and the relevant info API Gateway passes on: https://gist.github.com/GREsau/444c7d4d0bb9e8c82dd1a585931028bd
Note that this was tested with API Gateway V1, so it's possible V2 behaves differently

[DragonStuff]

Hey maintainers,
My apologies for the absolutely newbie question, but would @dbanty’s fork here be ready to try out? I tried using it but ran into the following error:

   Compiling rocket_lamb v0.6.0 (https://github.com/dbanty/rocket-lamb?branch=rocket-0.5#9f327b00)
error[E0277]: the trait bound `rocket::http::uri::Origin<'_>: From<&std::string::String>` is not satisfied
   --> /home/ubuntu/.cargo/git/checkouts/rocket-lamb-41362f35081257cb/9f327b0/src/handler.rs:101:32
    |
101 |                 let rocket = r.mount(&base_path, routes);
    |                                ^^^^^ the trait `From<&std::string::String>` is not implemented for `rocket::http::uri::Origin<'_>`
    |
    = note: required because of the requirements on the impl of `Into<rocket::http::uri::Origin<'_>>` for `&std::string::String`
    = note: required because of the requirements on the impl of `TryFrom<&std::string::String>` for `rocket::http::uri::Origin<'_>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: could not compile `rocket_lamb`

For reference, I’m also using rocket with version "0.5.0-dev".

rocket = { git = 'https://github.com/SergioBenitez/Rocket', rev = '263e39b5b429de1913ce7e3036575a7b4d88b6d7' }

And the fork for Rocket 0.5 like so.

rocket_lamb = { git = 'https://github.com/dbanty/rocket-lamb', branch = "rocket-0.5" }

I’ll try and fix it myself (using the handy explain function!), but I’m really new to Rust. Again my sincere apologies for this comment but you might be able to educate me on this, thanks for your time.

[dbanty]

@DragonStuff Rust is very particular about the way you use Git dependencies. If two places depend on the same Git-based crate they have to be declared exactly the same way. In this case, my fork declares Rocket like

rocket = { git = "https://github.com/SergioBenitez/Rocket", branch = "master" }

so you probably need to use exactly that in your Cargo.toml.

You can also check out my experiments repo where I have a few branches using this that all compiled and ran last I checked. The branch called rocket-0.5-CDK is probably the most up to date.

[sedrik]

I just tried @dbanty's fork with the latest 0.5.0-dev and got this error.

error[E0107]: missing generics for struct `Rocket`
   --> /home/sedrik/.cargo/git/checkouts/rocket-lamb-41362f35081257cb/9f327b0/src/lib.rs:62:20
    |
62  | impl RocketExt for Rocket {
    |                    ^^^^^^ expected 1 type argument
    |
note: struct defined here, with 1 type parameter: `P`
   --> /home/sedrik/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/f1ecb79/core/lib/src/rocket.rs:109:12
    |
109 | pub struct Rocket<P: Phase>(pub(crate) P::State);
    |            ^^^^^^ -
help: use angle brackets to add missing type argument
    |
62  | impl RocketExt for Rocket<P> {
    |                          ^^^

error[E0107]: missing generics for struct `Rocket`
   --> /home/sedrik/.cargo/git/checkouts/rocket-lamb-41362f35081257cb/9f327b0/src/builder.rs:14:13
    |
14  |     rocket: Rocket,
    |             ^^^^^^ expected 1 type argument
    |

I did not quite work out what the issue was though :(

I ran a modified version of https://github.com/GREsau/example-rocket-lamb-api with the following dependencies in Cargo.toml

[dependencies]
rocket = { git = "https://github.com/SergioBenitez/Rocket", branch = "master", default-features = false }
rocket_lamb = {git = "https://github.com/dbanty/rocket-lamb", branch= "rocket-0.5"}

[mwnDK1402]

I just tried @dbanty's fork with the latest 0.5.0-dev and got this error.

error[E0107]: missing generics for struct `Rocket`
   --> /home/sedrik/.cargo/git/checkouts/rocket-lamb-41362f35081257cb/9f327b0/src/lib.rs:62:20
    |
62  | impl RocketExt for Rocket {
    |                    ^^^^^^ expected 1 type argument
    |
note: struct defined here, with 1 type parameter: `P`
   --> /home/sedrik/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/f1ecb79/core/lib/src/rocket.rs:109:12
    |
109 | pub struct Rocket<P: Phase>(pub(crate) P::State);
    |            ^^^^^^ -
help: use angle brackets to add missing type argument
    |
62  | impl RocketExt for Rocket<P> {
    |                          ^^^

error[E0107]: missing generics for struct `Rocket`
   --> /home/sedrik/.cargo/git/checkouts/rocket-lamb-41362f35081257cb/9f327b0/src/builder.rs:14:13
    |
14  |     rocket: Rocket,
    |             ^^^^^^ expected 1 type argument
    |

I did not quite work out what the issue was though :(

I ran a modified version of https://github.com/GREsau/example-rocket-lamb-api with the following dependencies in Cargo.toml

[dependencies]
rocket = { git = "https://github.com/SergioBenitez/Rocket", branch = "master", default-features = false }
rocket_lamb = {git = "https://github.com/dbanty/rocket-lamb", branch= "rocket-0.5"}

The Rocket struct is now generic. It can be Rocket<Build>, Rocket<Ignite>, and Rocket<Orbit>, depending on which state it is in.

[mwnDK1402]

I've forked this, and I think I've managed to fix the few things that broke.

• I changed all Rocket to Rocket<Build>
• I changed Uri::percent_encode(...) to RawStr::new(...).percent_encode()

No more compilation errors from rocket_lamb, but I've yet to test it completely, because I still need to upgrade the rest of my application to Rocket 0.5.

[mwnDK1402]

I just submitted a pull request with the fixes.

[mwnDK1402]

In case someone needs the fix right away, here is my fork.

[dbanty]

Merged! Thanks @mwnDK1402!

[mwnDK1402]

I'm using Rocket 0.5 with @dbanty's fork, but I haven't tested this on older versions.

It says here that you can specify ROCKET_CLI_COLORS as off, but that makes rocket_lamb panic.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Config(Error { tag: Tag(Global, 3), profile: Some(Profile(Uncased { string: "global" })), metadata: Some(Metadata { name: "`ROCKET_` environment variable(s)", source: None, provide_location: Some(Location { file: "/root/.cargo/git/checkouts/rocket-8bf16d9ca7e90bdc/0d53e23/core/lib/src/config/config.rs", line: 213, col: 14 }), interpolater:  }), path: ["cli_colors"], kind: InvalidValue(Str("off"), "true or false"), prev: None })', rocket-lamb/src/handler.rs:103:49

[mwnDK1402]

And when I actually set it to ROCKET_CLI_COLORS=false or ROCKET_CLI_COLORS=0, it does launch, but I get logs like these in CloudWatch:

[WARN rocket::config::config] cli colors: �[1;49;39mfalse�[0m

[valpackett]

FYI looks like there is a fresh alternative to this crate in lambda_web. but this crate does have a cooler sounding name :)

[dbanty]

That's a really cool crate! Great that someone is moving the serverless tooling forward!

[sedrik]

I just want to point you all to this PR that I just made to support the rc1 of rocket 0.5 in @dbanty's fork. Please have a look if you are interested dbanty#2