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

HTML_UI should not panic on invalid input #18

Open
rimutaka opened this issue Sep 6, 2021 · 2 comments
Open

HTML_UI should not panic on invalid input #18

rimutaka opened this issue Sep 6, 2021 · 2 comments
Labels
bug Something isn't working

Comments

@rimutaka
Copy link
Member

rimutaka commented Sep 6, 2021

Sending an invalid search string from the front end may force the lambda to panic. E.g.

Sep 06 01:05:30.924  INFO stm_html_ui::handler: Raw path: /, Query: C++
Sep 06 01:05:30.924  INFO stm_html_ui::handler: Decoded path: /, query: C++, dev: None
Sep 06 01:05:30.925  INFO stm_html_ui::html: Terms: ["C++"]
Sep 06 01:05:30.925 ERROR stm_html_ui::elastic: Invalid field_value: c++
Sep 06 01:05:30.926 ERROR stm_html_ui::elastic: Invalid field_value: c++
Sep 06 01:05:30.926 ERROR stm_html_ui::elastic: Invalid field_value: c++
thread 'main' panicked at 'html() failed: ()', stm_html_ui/src/handler.rs:81:73
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/panicking.rs:92:14
   2: core::result::unwrap_failed
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/result.rs:1355:5
   3: core::result::Result<T,E>::expect
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/core/src/result.rs:997:23
   4: stm_html_ui::handler::my_handler::{{closure}}
             at ./src/handler.rs:81:21

The problem is in handler.rs unwrapping via expect :

let html_data = html::html(&config, url_path, url_query, dev).await.expect("html() failed");

There should be match and a meaningful error message returned to the user.

@rimutaka rimutaka added the bug Something isn't working label Sep 6, 2021
@rimutaka
Copy link
Member Author

It was changed to a better unwrap, but is still not ideal.

    let html_data = match html::html(&config, url_path, url_query, dev, api_request.headers).await {
        Ok(v) => v,
        Err(_) => return gw_response("Server Error".to_owned(), 500, 600),
    };

@rimutaka
Copy link
Member Author

rimutaka commented Jan 31, 2022

There is a difference between ignoring invalid input and returning 404 and recovering from an error and returning out own 5xx.

Panics are easy to track in Lambda monitoring. We should stop returning 500 and leave to the API GW.

If we are to return an error it should be informative to the end user.

rimutaka added a commit that referenced this issue Feb 1, 2022
* the handler returns an empty error struct for the runtime to record as an error and log in CloudWatch
* these errors should show up in monitoring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant