Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UnknownOperationException from API Gateway + Lambda using lambda_http 0.8.0 and lambda_runtime 0.8 #2676

Closed
anthonydandrea opened this issue May 5, 2023 · 4 comments
Labels
server Rust server SDK

Comments

@anthonydandrea
Copy link

I've created an API Gateway connected to a Rust Lambda following the Pokemon service guide. I've wrapped my Smithy service using the crates lambda_http = 0.8.0 and lambda_runtime = 0.8 like so:

  #[tokio::main]
  pub async fn main() {
     setup_tracing();

      let app = BrokerService::builder_without_plugins()                                                                                                                                                                         
          .do_nothing(do_nothing)
          .check_health(check_health)
          .build()
          .expect("failed to build an instance of BrokerService");

      let handler = LambdaHandler::new(app);
      let lambda = lambda_http::run(handler);

      if let Err(err) = lambda.await {
          eprintln!("lambda error: {}", err);
      }
  }

However, when I downgrade those lambda crates to 0.7.0 and 0.7 respectively, I receive a 200 response from both my routes.

@david-perez david-perez added the server Rust server SDK label May 5, 2023
@david-perez
Copy link
Contributor

Can reproduce. Adding two tower_http::trace::TraceLayer::new_for_http() layers, one before and one after wrapping the Service in LambdaHandler yields:

{"timestamp":"2023-05-05T17:03:53.336874Z","level":"DEBUG","fields":{"message":"started processing request"},"target":"tower_http::trace::on_request","span":{"method":"GET","uri":"https://redacted.execute-api.us-east-1.amazonaws.com/prod/do-nothing","version":"HTTP/1.1","name":"request"},"spans":[{"requestId":"bc321be8-58b7-4dfb-affe-255427fbd2ff","xrayTraceId":"Root=1-645536f9-2a73cf0f49c91ea55799e5d1;Parent=704b2a5c2460b6c7;Sampled=0","name":"Lambda runtime invoke"},{"method":"GET","uri":"https://redacted.execute-api.us-east-1.amazonaws.com/prod/do-nothing","version":"HTTP/1.1","name":"request"}]}
{"timestamp":"2023-05-05T17:03:53.336912Z","level":"DEBUG","fields":{"message":"started processing request"},"target":"tower_http::trace::on_request","span":{"method":"GET","uri":"https://redacted.execute-api.us-east-1.amazonaws.com/prod/do-nothing","version":"HTTP/1.1","name":"request"},"spans":[{"requestId":"bc321be8-58b7-4dfb-affe-255427fbd2ff","xrayTraceId":"Root=1-645536f9-2a73cf0f49c91ea55799e5d1;Parent=704b2a5c2460b6c7;Sampled=0","name":"Lambda runtime invoke"},{"method":"GET","uri":"https://redacted.execute-api.us-east-1.amazonaws.com/prod/do-nothing","version":"HTTP/1.1","name":"request"},{"method":"GET","uri":"https://redacted.execute-api.us-east-1.amazonaws.com/prod/do-nothing","version":"HTTP/1.1","name":"request"}]}

which seems to indicate convert_event in lambda_handler.rs is not stripping away the /prod stage prefix:

https://github.com/awslabs/smithy-rs/blob/74b2df9337837f7858cf8f3e736bd9f43205d9a3/rust-runtime/aws-smithy-http-server/src/routing/lambda_handler.rs#L56-L89

This change in lambda_http in between versions 0.7 and 0.8 is interesting: awslabs/aws-lambda-rust-runtime#607

Best course of action to continue investigating this would be to instrument convert_event.

@david-perez
Copy link
Contributor

Instrumenting convert_event revealed that indeed the problem arises from that PR. That PR made it so that the RawHttpPath type is different between the v0.8.0 and the v0.7.0 versions of the lambda_http crate.

  • In v0.7.x, the type was found at lambda_http::ext::RawHttpPath.
  • Starting from v0.8.0, the type is now found at lambda_http::ext::extensions::RawHttpPath.

These are, to the eyes of the compiler, fundamentally different types with no relation whatsoever.

So what happened here, with a smithy-rs service depending on the lambda_http = "0.8.0" crate and the aws-smithy-http-server = { version = "0.55.2", features = ["aws-lambda"] } crate (which depends on lambda_http = "^0.7.0"), is that the two versions of lambda_http were in the service's closure, and the request flowed like this:

  1. lambda_http 0.8.0 receives the request from API Gateway in the form of a Lambda event and inserts the lamba_http::ext::extensions::RawHttpPath in the HTTP extensions bag when converting it to an http::Request.
  2. The http::Request is handed over to the LambdaHandler service, which invokes aws_smithy_http_server::routing::lambda_handler::convert_event to convert the lambda_http::Request into a http::Request<hyper::Body>.

https://github.com/awslabs/smithy-rs/blob/97c70c429c9bb2a3885d3487e22fd24adaf12d1c/rust-runtime/aws-smithy-http-server/src/routing/lambda_handler.rs#L56-L61

  1. While converting the event, this function attempts to remove the API Gateway Stage portion of the URI. This is done in the call to let raw_path = request.raw_http_path();, which uses lambda_http ^0.7.0 to retrieve the lambda_http::ext::RawHttpPath type from the extensions bag.
fn raw_http_path(&self) -> String {
    self.extensions()
        .get::<RawHttpPath>()
        .map(|ext| ext.0.clone())
        .unwrap_or_default()
}

This of course fails, since that type was not inserted by lambda_http v0.8.0 before. So the unwrap_or_default() gets enacted, and an empty String is returned. This makes convert_event return the incoming request as-is, without stripping off the stage from the URI. So if curl -X POST -v https://redacted.execute-api.us-east-1.amazonaws.com/prod/operation was hit, then /prod/operation is routed by smithy-rs, which hence yields an UnknownOperationException, since the router has the route registered at POST /operation.

david-perez added a commit that referenced this issue May 9, 2023
This patch also removes the unneeded dependency on `lambda_runtime` by
`aws-smithy-http-server-python`.

This patch also refactors `LambdaHandler`'s `convert_event` to avoid
cloning the URI path when not needed.

This is a breaking change. See #2676 why.
@david-perez
Copy link
Contributor

Ultimately, we've decided to palliate this shortcoming with documentation in #2683. See the linked PRs/issues for relevant tracked work.

david-perez added a commit that referenced this issue May 10, 2023
…#2683)

See #2676.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 10, 2023
…2685)

This patch also removes the unneeded dependency on `lambda_runtime` by
`aws-smithy-http-server-python`.

This patch also refactors `LambdaHandler`'s `convert_event` to avoid
cloning the URI path when not needed.

This is a breaking change. See #2676 why.

This patch also bumps `aws-smithy-http-server` dependency on `mime` to
0.3.4.

`cargo +nightly-2022-11-16 minimal-versions check --all-features`
otherwise
fails when using 0.3.0, because we require `impl fmt::Display for
mime::FromStrError`, which was first introduced in 0.3.4.

As to why `minimal-versions` is now picking `mime` 0.3.0 with this
patch, it's
because in `lambda_http` 0.7.3, they had [`mime =

"0.3.16"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/99dba6447253ac87cf3cefeb2ba130b50514f9df/lambda-http/Cargo.toml#L35-L35),
and in `lambda_http` 0.8.0, they've now relaxed that to [`mime =

"0.3"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/393d6447bea0502e1f939d197f4facc228e6e007/lambda-http/Cargo.toml#L36).


## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 10, 2023
…2685)

This patch also removes the unneeded dependency on `lambda_runtime` by
`aws-smithy-http-server-python`.

This patch also refactors `LambdaHandler`'s `convert_event` to avoid
cloning the URI path when not needed.

This is a breaking change. See #2676 why.

This patch also bumps `aws-smithy-http-server` dependency on `mime` to
0.3.4.

`cargo +nightly-2022-11-16 minimal-versions check --all-features`
otherwise
fails when using 0.3.0, because we require `impl fmt::Display for
mime::FromStrError`, which was first introduced in 0.3.4.

As to why `minimal-versions` is now picking `mime` 0.3.0 with this
patch, it's
because in `lambda_http` 0.7.3, they had [`mime =

"0.3.16"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/99dba6447253ac87cf3cefeb2ba130b50514f9df/lambda-http/Cargo.toml#L35-L35),
and in `lambda_http` 0.8.0, they've now relaxed that to [`mime =

"0.3"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/393d6447bea0502e1f939d197f4facc228e6e007/lambda-http/Cargo.toml#L36).


## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 18, 2023
…#2683)

See #2676.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 18, 2023
…2685)

This patch also removes the unneeded dependency on `lambda_runtime` by
`aws-smithy-http-server-python`.

This patch also refactors `LambdaHandler`'s `convert_event` to avoid
cloning the URI path when not needed.

This is a breaking change. See #2676 why.

This patch also bumps `aws-smithy-http-server` dependency on `mime` to
0.3.4.

`cargo +nightly-2022-11-16 minimal-versions check --all-features`
otherwise
fails when using 0.3.0, because we require `impl fmt::Display for
mime::FromStrError`, which was first introduced in 0.3.4.

As to why `minimal-versions` is now picking `mime` 0.3.0 with this
patch, it's
because in `lambda_http` 0.7.3, they had [`mime =

"0.3.16"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/99dba6447253ac87cf3cefeb2ba130b50514f9df/lambda-http/Cargo.toml#L35-L35),
and in `lambda_http` 0.8.0, they've now relaxed that to [`mime =

"0.3"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/393d6447bea0502e1f939d197f4facc228e6e007/lambda-http/Cargo.toml#L36).


## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 22, 2023
…#2683)

See #2676.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 22, 2023
…2685)

This patch also removes the unneeded dependency on `lambda_runtime` by
`aws-smithy-http-server-python`.

This patch also refactors `LambdaHandler`'s `convert_event` to avoid
cloning the URI path when not needed.

This is a breaking change. See #2676 why.

This patch also bumps `aws-smithy-http-server` dependency on `mime` to
0.3.4.

`cargo +nightly-2022-11-16 minimal-versions check --all-features`
otherwise
fails when using 0.3.0, because we require `impl fmt::Display for
mime::FromStrError`, which was first introduced in 0.3.4.

As to why `minimal-versions` is now picking `mime` 0.3.0 with this
patch, it's
because in `lambda_http` 0.7.3, they had [`mime =

"0.3.16"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/99dba6447253ac87cf3cefeb2ba130b50514f9df/lambda-http/Cargo.toml#L35-L35),
and in `lambda_http` 0.8.0, they've now relaxed that to [`mime =

"0.3"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/393d6447bea0502e1f939d197f4facc228e6e007/lambda-http/Cargo.toml#L36).


## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 22, 2023
…#2683)

See #2676.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
david-perez added a commit that referenced this issue May 22, 2023
…2685)

This patch also removes the unneeded dependency on `lambda_runtime` by
`aws-smithy-http-server-python`.

This patch also refactors `LambdaHandler`'s `convert_event` to avoid
cloning the URI path when not needed.

This is a breaking change. See #2676 why.

This patch also bumps `aws-smithy-http-server` dependency on `mime` to
0.3.4.

`cargo +nightly-2022-11-16 minimal-versions check --all-features`
otherwise
fails when using 0.3.0, because we require `impl fmt::Display for
mime::FromStrError`, which was first introduced in 0.3.4.

As to why `minimal-versions` is now picking `mime` 0.3.0 with this
patch, it's
because in `lambda_http` 0.7.3, they had [`mime =

"0.3.16"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/99dba6447253ac87cf3cefeb2ba130b50514f9df/lambda-http/Cargo.toml#L35-L35),
and in `lambda_http` 0.8.0, they've now relaxed that to [`mime =

"0.3"`](https://github.com/awslabs/aws-lambda-rust-runtime/blob/393d6447bea0502e1f939d197f4facc228e6e007/lambda-http/Cargo.toml#L36).


## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
@david-perez
Copy link
Contributor

I'm going to go ahead and close this since the documentation has gone out in smithy-rs 0.56.0. smithy-rs 0.56.0 now works with lambda_http ^0.8.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Rust server SDK
Projects
None yet
Development

No branches or pull requests

2 participants