stackmuncher / stm_server

The server-side code behind stackmuncher.com, a global directory of software developers.
https://stackmuncher.com
GNU Affero General Public License v3.0
2 stars 0 forks source link

HTML_UI should not panic on invalid input #18

Open rimutaka opened 3 years ago

rimutaka commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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.