tokio-rs / axum

Ergonomic and modular web framework built with Tokio, Tower, and Hyper
19.03k stars 1.06k forks source link

Macro for JsonDeserializer extractor to avoid boilerplate code #2849

Closed manuelgdlvh closed 3 months ago

manuelgdlvh commented 3 months ago

Macro for the JsonDeserializer extractor to avoid boilerplate code in most situations where the handler returns a Response or its derivatives, and returns early in case of failure.

Motivation

In most situations, the controller layer is responsible for managing the result of the service layer and returning a specific response (not an expected result). Given this scenario, using the JsonDeserializer extractor, we should handle both when the payload is correct and when it is not. This macro returns the expected payload, and if not, it returns early a Response structure, avoiding duplicate code.

Solution

A macro that returns the payload if everything went well, otherwise it returns the error as a Response structure that implements IntoResponse.

jplatte commented 3 months ago

I don't think this makes a lot of sense. Have you considered changing your handler to return axum::response::Result<JsonDeserializer<T>>, call .deserialize()? and wrapping your other return values in Ok()?

manuelgdlvh commented 3 months ago

Hi @jplatte , i made this small change to avoid exactly that, put Ok() and Result<, _> in all handlers considering that handlers must to manage all resulting of below layers and prepare each type of response. Something like bail! macro (more or less) in anyhow crate does to keep less boilerplate!

jplatte commented 3 months ago

Well, using ? is usually much easier than macros for early returns. It's not really expected that many handlers return exactly a Response so I don't think this macro is useful for many users of axum.

If you want, you can use the Discussions section to discuss the merit of this kind of macro or your style of writing handlers more generally, but I'm going to close this PR because I'm fairly certain it doesn't make sense for axum to include this macro.

Thanks for the PR anyways though! If you have other ideas for how to improve axum, feel free to open issues or discussions about it, or send a PR if it's not much effort.