actix / actix-web

Actix Web is a powerful, pragmatic, and extremely fast web framework for Rust.
https://actix.rs
Apache License 2.0
21.6k stars 1.67k forks source link

JSON Deserialization errors are no longer outputted by default #1237

Closed Darkeye9 closed 4 years ago

Darkeye9 commented 4 years ago

Hi all and extensive congratulation on putting v2.0.0 out! Support for async/await and updated dependencies were all very welcomed by our team.

I wanted to raise a question, more than a bug. I noticed, that in the first project were are doing with the updated actix_web 2.0.0, JSON deserialization errors that used to be included in the HTTP 400 responses are now gone.

We did not change any of our code handling errors and neither it seems that we were changing actix-web native error handling, so I am supposing it has to do with the new release, as we were tracking new releases of actix-web, and only now I am missing those messages.

For reference, the messages I am talking about are the ones that occur when a JSON body does not conform to some input model an endpoint requires. They were along the lines of:

Json deserialize error: missing field `description` at line 3 column

I set the project logging configuration to TRACE, and I could not see any JSON validation error. So this seems like a developer-unfriendly failure here. By default, the HTTP error messages should come back, or a log line should be outputted, for developer sanity.

Had a good christmas and tell me what you think! :santa:

fafhrd91 commented 4 years ago

std::env::set_var("RUST_LOG", "actix_web=trace");

[2019-12-26T17:13:21Z DEBUG actix_web::types::json] Failed to deserialize Json from payload. Request path: /extractor
[2019-12-26T17:13:21Z DEBUG actix_web::middleware::logger] Error in response: Deserialize(Error("EOF while parsing a value", line: 1, column: 0))
[2019-12-26T17:13:21Z INFO  actix_web::middleware::logger] 127.0.0.1:52676 "POST /extractor HTTP/1.1" 400 0 "-" "curl/7.64.1" 0.000573
Darkeye9 commented 4 years ago

Ok, I can now see that message. I am amazed by that not being in the error level, or at least, something higher than debug.

Is the output in the HTTP Response silenced on purpose in this release? And can I enable it back?

Greetings and thank you for your time ;)

fafhrd91 commented 4 years ago

Nothing changed since 1.0 release

fafhrd91 commented 4 years ago

You can build your own extractor, it is easy

surfingtomchen commented 4 years ago

Hi,

I just find the extractor way to solve this issue is a little bit complicated, because I need to write multiple extractors for each JSON structure. For example:

there is a user structure in user.rs:

pub struct User {
    pub _id: Option<ObjectId>,
    pub name: String,
    pub password: String,
}

error.rs:

impl ApiError {

    pub fn json_error(cfg: JsonConfig) -> JsonConfig {
        cfg.limit(4096).error_handler(|err: JsonPayloadError, _req| {
            // create custom error response
            error::InternalError::from_response(
                format!("JSON error: {:?}", err),
                ApiError::JsonError { cause: format!("{:?}", err) }.error_response(),
            ).into()
        })
    }
}

main.rs:

    HttpServer::new(||
        {
            App::new()
                .service(misc_handler::health_check)
                .service(
                    web::scope("/api/v1")
                        // change json extractor configuration
                        .app_data(web::Json::<User>::configure(|cfg| {
                            ApiError::json_error(cfg)
                        }))
                        .route("/user/save", web::post().to(user_handler::save_user))
                )
        })
        .bind("127.0.0.1:8088")?
        .run()
        .await

if I have another structure, I need to write the extractor again before that route. Is there any convenience way to solve this? If I just want to return the normal json deserialization error such as the format error, missing "," etc.

Neopallium commented 4 years ago

You should be able to add a JsonConfig for all routes in that scope. This way you don't need to do it for every json type.

    HttpServer::new(||
        {
            App::new()
                .service(misc_handler::health_check)
                .service(
                    web::scope("/api/v1")
                        // One JsonConfig for all routes in this scope.
                        .app_data(ApiError::json_error(JsonConfig::default()))
                        .route("/user/save", web::post().to(user_handler::save_user))
                )
        })
        .bind("127.0.0.1:8088")?
        .run()
        .await
surfingtomchen commented 4 years ago

You should be able to add a JsonConfig for all routes in that scope. This way you don't need to do it for every json type.

    HttpServer::new(||
        {
            App::new()
                .service(misc_handler::health_check)
                .service(
                    web::scope("/api/v1")
                        // One JsonConfig for all routes in this scope.
                        .app_data(ApiError::json_error(JsonConfig::default()))
                        .route("/user/save", web::post().to(user_handler::save_user))
                )
        })
        .bind("127.0.0.1:8088")?
        .run()
        .await

thanks for your prompt reply!