awslabs / aws-lambda-rust-runtime

A Rust runtime for AWS Lambda
Apache License 2.0
3.35k stars 342 forks source link

Query Parameter is only deserialized up to the first parameter and all the rest are left as None #802

Closed Bryson14 closed 9 months ago

Bryson14 commented 9 months ago

I am using Axum rust to handle Query parameters in the URL within the runtime. The issue that I can't figure out is that the program will only deserialize the first query parameter. If there are two or more, then the second and third query parameters will be read as null.

#![warn(clippy::cargo, clippy::unwrap_used)]
mod config;
mod models;
mod route_handlers;
mod web_server;
use aws_sdk_dynamodb::Client;
use config::{make_config, Opt};
use lambda_runtime::tower::ServiceBuilder;
use std::env::set_var;
use tower_http::add_extension::AddExtensionLayer;
use web_server::get_router;

const REGION: &str = "us-east-1";

#[tokio::main]
async fn main() -> Result<(), lambda_http::Error> {
    // AWS Runtime can ignore Stage Name passed from json event
    // https://github.com/awslabs/aws-lambda-rust-runtime/issues/782
    set_var("AWS_LAMBDA_HTTP_IGNORE_STAGE_IN_PATH", "true");

    // required to enable CloudWatch error logging by the runtime
    tracing_subscriber::fmt()
        .with_max_level(tracing::Level::INFO)
        // disable printing the name of the module in every log line.
        .with_target(false)
        // disabling time is handy because CloudWatch will add the ingestion time.
        .without_time()
        .init();

    // creating a dynamo db client for the router to use
    let config = make_config(Opt {
        region: Some(REGION.to_owned()),
        verbose: true,
    })
    .await;
    let client = std::sync::Arc::new(Client::new(
        &config.expect("Could not unwrap aws config for dynamodb client"),
    ));

    let router = get_router();

    // adding the client into the router as an extension layer
    let router = ServiceBuilder::new()
        .layer(AddExtensionLayer::new(client))
        .service(router);

    lambda_http::run(router).await
}

pub fn get_router() -> axum::Router {
    // Sets the axum router to produce only ERROR level logs. The handlers can produce their own level of logs.
    const LOG_LEVEL: Level = Level::ERROR;

    // With Routing, order does matter, so put the more specific first and then more generic routes later
    Router::new()
        .route("/conversations", get(list_conversations));

pub async fn list_conversations(
    Extension(db_client): Extension<Arc<DynamoDbClient>>,
    Query(params): Query<PaginationRequest>,
) -> WebResult {
    info!("params: {:?}", params);
    let client = db_client.clone();
    scan_items::<Conversation>(&client, DBTable::Conversation, params).await
}

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct PaginationRequest {
    // The hash key
    pub next_key_a: Option<String>,

    // The sort key
    pub next_key_b: Option<String>,

    // if no pagination should be used for the client, and all data should be returned
    pub fetch_all: Option<bool>,
}

When I invoke the api gateway, with next_key_a first, it will be read but next_key_b will be None. If i switch the query parameters, they will then change from Some to None

Calling https://5oclxtbcq3.execute-api.us-east-1.amazonaws.com/test-stage/conversations?next_key_b=7e95e561-3b3f-4a82-954f-fb37f8c0f257&next_key_a=053baf43-ac9c-4754-a94a-de982c169204 will cause next_key_b to be Some() and next_key_a to be None.

Then calling https://5oclxtbcq3.execute-api.us-east-1.amazonaws.com/test-stage/conversations?next_key_a=7e95e561-3b3f-4a82-954f-fb37f8c0f257&next_key_b=053baf43-ac9c-4754-a94a-de982c169204 will be the opposite.

I think it is something that happens before the payload from API Gateway hits the axum web server because I've tried a bunch of unit tests to make sure this struct can handle this from the url:

 #[test]
    fn test_pagination_request_full() {
        let uri: Uri = "http://example.com/path?next_key_a=hello&next_key_b=world&fetch_all=true"
            .parse()
            .unwrap();
        let result: Query<PaginationRequest> = Query::try_from_uri(&uri).unwrap();
        assert_eq!(result.0.next_key_a, Some(String::from("hello")));
        assert_eq!(result.0.next_key_b, Some(String::from("world")));
        assert_eq!(result.0.fetch_all, Some(true));
    }

    #[test]
    fn test_pagination_request_full_swapped() {
        let uri: Uri = "http://example.com/path?next_key_b=world&fetch_all=true&next_key_a=hello"
            .parse()
            .unwrap();
        let result: Query<PaginationRequest> = Query::try_from_uri(&uri).unwrap();
        assert_eq!(result.0.next_key_a, Some(String::from("hello")));
        assert_eq!(result.0.next_key_b, Some(String::from("world")));
        assert_eq!(result.0.fetch_all, Some(true));
    }

    #[test]
    fn test_pagination_request_partial() {
        let uri: Uri = "http://example.com/path?next_key_a=hello&fetch_all=true"
            .parse()
            .unwrap();
        let result: Query<PaginationRequest> = Query::try_from_uri(&uri).unwrap();
        assert_eq!(result.0.next_key_a, Some(String::from("hello")));
        assert_eq!(result.0.next_key_b, None);
        assert_eq!(result.0.fetch_all, Some(true));
    }
calavera commented 9 months ago

can you access the Request object in Axum and check if the request.uri() has all the parameters correctly?

calavera commented 9 months ago

I'm not sure how Axum transforms requests into Query struct, but I believe the runtime is generating the right Request object that we're sending to the frameworks. The tests in #803 show that the Request has all the query parameters, even with duplicated values.

calavera commented 9 months ago

I added tests in #803 that show that Axum can correctly extract the query parameters from our Requests objects.

I noticed that Axum has two different versions of the extractors. I don't know which one you're using but the one in axum-extra seems to be more correct. See this comment: https://github.com/tokio-rs/axum/blob/main/axum/src/extract/query.rs#L38-L49

calavera commented 9 months ago

We've added tests in #803 and #804 that show that the runtime integration with Axum is doing the right thing. It's possible that you APIGW configuration has something to do with this.

Let me know if you've found something on your end, otherwise, we'll close this as a non issue in a couple of days.

Bryson14 commented 9 months ago

Thanks for the update, I'll be able to confirm this tomorrow.

Bryson14 commented 9 months ago

Here is what I have so far, I have a catch all route to try this out that looks like thi:

        .route("/*path", get(catch_get).post(catch_post))
}

/// A catch all route for GET routes for helpful debugging from the client side and API gateway
async fn catch_get(req: Request<Body>) -> JsonResponse {
    let uri = req.uri();
    let res = ErrorResponse {
        error_message: &format!(
            "unknown GET request to route '{uri}'. Check your path and try again. 
    If there is a extra url parameter passed on by API Gateway, 
    you can use the environment variable 'STAGE_NAME' to compensate for it."
        ),
    };
    info!("Unknown request caught. Response: {:?}", res);
    (StatusCode::NOT_FOUND, Json(res.to_value()))
}

When invoking from the lambda test window with this json, I get no path params. I assume because lambda runtime has to interperet the json object into a url request

{
  "path": "/conversations/idk/c8089eae-5e7d-11ec-94c8-b42e99b5b2c6?next_key_a=123",
  "httpMethod": "GET",
  "headers": {
    "Accept": "*/*",
    "Content-Type": "application/json"
  }
}

So this json payload results in 404 :

{
  "statusCode": 404,
  "headers": {
    "content-type": "application/json",
    "content-length": "282"
  },
  "multiValueHeaders": {
    "content-type": [
      "application/json"
    ],
    "content-length": [
      "282"
    ]
  },
  "body": "{\"error_message\":\"unknown GET request to route '/conversations/idk/c8089eae-5e7d-11ec-94c8-b42e99b5b2c6'. Check your path and try again. \\n    If there is a extra url parameter passed on by API Gateway, \\n    you can use the environment variable 'STAGE_NAME' to compensate for it.\"}",
  "isBase64Encoded": false
}

From Cloud Shell

Since api gateway doesn't support testing since I just have a single resouce with /{+proxy} pointing towards my lambda, I have to use the shell to invoke the API

curl https://123abc.execute-api.us-east-1.amazonaws.com/test-stage/what?limit=1&okay=why&hello=world&thanks=true leads to :

{"error_message":"unknown GET request to route 'https://5oclxtbcq3.execute-api.us-east-1.amazonaws.com/what?limit=1'. Check your path and try again. \n    If there is a extra url parameter passed on by API Gateway."}

Removing the first query parameter leads curl https://123abc.execute-api.us-east-1.amazonaws.com/test-stage/what?okay=why&hello=world&thanks=true leads to :

{"error_message":"unknown GET request to route 'https://5oclxtbcq3.execute-api.us-east-1.amazonaws.com/what?okay=why'. Check your path and try again. \n    If there is a extra url parameter passed on by API Gateway."}

Maybe the +proxy from api gateway doesn't pass along all the query params?

Screenshot of the proxy resource and api gateway

image

Bryson14 commented 9 months ago

I was playing around more with the lambda console and sending json payloads directly:

This payload was interpreted correctly:

{
  "path": "/conversations/idk/c8089eae-5e7d-11ec-94c8-b42e99b5b2c6?next_key_a=123",
  "queryStringParameters": {
      "hello": "world"
  },
  "httpMethod": "GET",
  "headers": {
    "Accept": "*/*",
    "Content-Type": "application/json"
  }
}

Adding two or three string query parameters was read into the lambda correctly as well. However, when trying to pass in a number as a query parameter, I got a lambda-runtime error:

{
  "path": "/conversations/idk/c8089eae-5e7d-11ec-94c8-b42e99b5b2c6?next_key_a=123",
  "queryStringParameters": {
      "hello": "world",
      "name": "me",
      "test": 1
  },
  "httpMethod": "GET",
  "headers": {
    "Accept": "*/*",
    "Content-Type": "application/json"
  }
}
{
  "errorType": "&lambda_runtime::deserializer::DeserializeError",
  "errorMessage": "failed to deserialize the incoming data into the function's payload type: this function expects a JSON payload from Amazon API Gateway, Amazon Elastic Load Balancer, or AWS Lambda Function URLs, but the data doesn't match any of those services' events\n"
}

Perhaps keeping numbers is intentional part of the lambda runtime...

But I finally got the lambda function to work correctly when passing in the payload like this:

{
  "path": "/conversations",
  "queryStringParameters": {
      "limit": "1",
      "next_key_a":"7e95e561-3b3f-4a82-954f-fb37f8c0f257",
      "next_key_b":"675fc382-9dd7-4fe0-8c5b-c98089e525ce"
  },
  "httpMethod": "GET",
  "headers": {
    "Accept": "*/*",
    "Content-Type": "application/json"
  }
}

I this means that API Gateway is handling the request in a way we are not expecting.

calavera commented 9 months ago

Adding two or three string query parameters was read into the lambda correctly as well. However, when trying to pass in a number as a query parameter, I got a lambda-runtime error:

This is correct. Query string parameters MUST be string. This is definitely what API Gateway sends in normal circumstances if you send HTTP requests to the endpoint that it generates for you. What you're basically experiencing is that the Lambda console generates invalid payloads and sends them to the Lambda function directly. I've seen this happening before. We cannot do much about it unfortunately.

To be honest, I have very little experience configuring API Gateway, so I cannot help you much. @bnusunny might know if that /{+proxy} pattern makes sense or not.

calavera commented 9 months ago

I believe this is not related to the runtime. I just deployed the axum example behind an APIGW rest api and it works just fine:

image

I used the CDK stack in https://github.com/awslabs/aws-lambda-rust-runtime/pull/807.

github-actions[bot] commented 9 months ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one.