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

Return HttpResponse object to chain methods #1224

Closed
omar-mohamed-khallaf opened this issue Apr 23, 2022 · 3 comments
Closed

Return HttpResponse object to chain methods #1224

omar-mohamed-khallaf opened this issue Apr 23, 2022 · 3 comments

Comments

@omar-mohamed-khallaf
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I've seen a continuous pattern in my code where I create the HttpResponse and then set the status code to return a response like:

Json::Value jsonResponse;
jsonResponse["err"] = "content-type is not application/json";

auto httpResponse = drogon::HttpResponse::newHttpJsonResponse(jsonResponse);
httpResponse->setStatusCode(drogon::k400BadRequest);
callback(httpResponse);

Describe the solution you'd like
Wouldn't it be much nicer if we could write it as:

Json::Value jsonResponse;
jsonResponse["err"] = "content-type is not application/json";
callback(drogon::HttpResponse::newHttpJsonResponse(jsonResponse)->setStatusCode(drogon::k400BadRequest));

and the same for headers:

callback(drogon::HttpResponse::newHttpJsonResponse(jsonResponse)
    ->addHeader("Example-Header-1", "value")
    ->addHeader("Example-Header-2", "value")
    ->addHeader("Example-Header-3", "value")
    ->addHeader("Example-Header-4", "value"));

These methods return void so I don't think they will break the API consistency, am I right?

@omar-mohamed-khallaf omar-mohamed-khallaf changed the title Return HttpResponse object to chain methods [enhancement] Return HttpResponse object to chain methods Apr 23, 2022
@omar-mohamed-khallaf omar-mohamed-khallaf changed the title [enhancement] Return HttpResponse object to chain methods Return HttpResponse object to chain methods Apr 23, 2022
@rbugajewski
Copy link
Collaborator

Thanks for your ideas!

As returning errors as JSON payload is not standardized (but there are certainly enough implementations out there that use it that way), personally I would have written a helper function, or even keep a global JSON bad request response, so that it could be reused.

I think I may see what you are trying to achieve here, but I think it is more of a cosmetic nature, or doing things in a different way.

In my opinion there should not be different interfaces for basically the same behavior, as this will cause confusion for framework users. I rather prefer a sanctioned way. I think this kind of changes would fit best into the Drogon 2.0 release cycle where we have the chance to introduce API breaking changes.

We already have issue #682, and #806 for API breaking changes. I think this open discussion better fits at these places.

@omar-mohamed-khallaf
Copy link
Contributor Author

Great, should I close this issue ?

@rbugajewski
Copy link
Collaborator

Sure, feel free to close it, as it should be already referenced in the other two issues 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants