BREAKING: add ContextFlowable too demand GetDefaultStatusCode #376
+47
−69
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates to #375
Please read the research done in #375.
Looking for some feedback. I looked at how echo/gin deal with this and this is pretty much it. You must wait until you set a
Content-Type
before you can callWriteHeader
asWriteHeader
sends data to the client to prepare for serialization of whatever payload we're calling.The PR removes the need to implement a separate SetDefaultStatusCode from all adaptors and centralized how we set our status code getting rid of some redundant code. I did take a shortcut just get it "working" by just wrapping up the code in our Send method. I think it's best we update our Sender functions to adhere to this signature:
type Sender func(http.ResponseWriter, *http.Request, int, any) error
. Assuming the rest of the idea his fine. Again I'm not really sure there is another way to do this while maintaining the current fuego.Server Serialization pattern.Side note: There is technically no way in
fuego.Server
to set a StatusCode on a response for say like204
or201
without using the DefaultStatusCode thing. Right now you can callctx.SetStatus
, but again that's the bug. Once you callWriteHeader
the Headers are written to the client. So say if OutTransform fails or some other part of our flow fails you'll only ever get what was Set by status first. Other frameworks go as far as even giving warning to their callers, like from echo:if WriteHeader is called twice when using echo it will actually warn with
"response already committed"
Sorry for all the words. This was pretty great learning experience