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

Content-Type returned by JSON is not application/json when used in special middlewares #78

Open
aharbis opened this issue Aug 19, 2022 · 3 comments
Labels
question Further information is requested

Comments

@aharbis
Copy link

aharbis commented Aug 19, 2022

💡 The feature or bug you are proposing

Using bunrouter.JSON in handlers passed to either WithNotFoundHandler or WithMethodNotAllowedHandler results in application/text Content-Type header.

📄 The description of the bug or the logic behind your proposal

Constructing my router like this:

router := bunrouter.New(
	bunrouter.WithMiddleware(reqLogMiddleware),
	bunrouter.WithMiddleware(errorHandlerMiddleware),
	bunrouter.WithNotFoundHandler(notFoundHandler),
	bunrouter.WithMethodNotAllowedHandler(methodNotAllowedHandler),
)

Both of my notFoundHandler and methodNotAllowedHandler middlewares send a valid JSON response with:

bunrouter.JSON(w, err)

Where err is a custom HTTPError type with json marshaling tags.

The JSON marshaling does work and the response received on the client is something like:

{"code":"not_found","message":"no route found for /"}

But the Content-Type header received is text/plain; charset=utf-8.

🚀 The expected result

bunrouter.JSON should always set application/json as the Content-Type, even when used in special middlewares.

@aharbis aharbis changed the title Content-Type returned by JSON is application/text when used in special middlewares Content-Type returned by JSON is not application/json when used in special middlewares Aug 19, 2022
@aharbis
Copy link
Author

aharbis commented Aug 19, 2022

Actually, this may not be scoped to just those middlewares. I am also seeing similar behavior for other routes, where neither of the NotFound nor MethodNotFound handlers were involved.

@aharbis
Copy link
Author

aharbis commented Aug 19, 2022

I think I found the root cause, note this comment in http.ResponseWriter:

// Changing the header map after a call to WriteHeader (or
// Write) has no effect unless the HTTP status code was of the
// 1xx class or the modified headers are trailers.

However in the bunrouter documentation, and indeed my code, the following pattern is used:

w.WriteHeader(httpErr.statusCode)
_ = bunrouter.JSON(w, httpErr)

This places the call to w.Header().Set() (inside bunrouter.JSON()) after w.WriteHeader(), which prevents the Content-Type from being set as expected.

@vmihailenco
Copy link
Member

vmihailenco commented Aug 19, 2022

Yes, you must be careful with WriteHeader. It is not specific to BunRouter and BunRouter does not offer a universal solution for this problem.

https://bunrouter.uptrace.dev/guide/golang-http-error-handling.html may give you some ideas. Also see uptrace for a more realistic example.

@vmihailenco vmihailenco added the question Further information is requested label Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants