feat(plugins): Introduce Goyave as middleware #203
Conversation
darkweak
added
enhancement
|
|
||
| // Write will write the response body | ||
| func (g *goyaveWriterDecorator) Write(b []byte) (int, error) { | ||
| g.goyaveResponse.WriteHeader(g.Response.StatusCode) |
There was a problem hiding this comment.
I don't think this line is necessary as goyave.Response already writes the header automatically.
There was a problem hiding this comment.
If I don't do that, there is a superfluous write header warning.
There was a problem hiding this comment.
Oh I see why, you are writing directly to the http.ResponseWriter in the line just below, so the goyave.Response doesn't know something has been written and tries to write the header again. You should always write to goyave.Response.
| if g.Response.Body != nil { | ||
| b, _ = ioutil.ReadAll(g.Response.Body) | ||
| } |
There was a problem hiding this comment.
Write may be called multiple times (typically when streaming a response). This may cause problems.
There was a problem hiding this comment.
If the response is a stream/chunked it won't be stored to avoid cache corruption at the moment. So it doesn't go through the write method.
| return len(b), nil | ||
| } | ||
|
|
||
| func (g *goyaveWriterDecorator) PreWrite(b []byte) { |
There was a problem hiding this comment.
PreWrite may never be called if nothing is written in the response (empty response, 204 No Content).
Close() is always called so you can add a check in there as well.
There was a problem hiding this comment.
I tried to return a no-content response and it has been triggered. 🤔
darkweak
force-pushed
the
feat/plugins/introduce-goyave-plugin
branch
from
575fab4
to
98088c3
Compare
darkweak
force-pushed
the
feat/plugins/introduce-goyave-plugin
branch
2 times, most recently
from
c730b22
to
0381ccd
Compare
darkweak
force-pushed
the
feat/plugins/introduce-goyave-plugin
branch
from
0381ccd
to
0fc14c9
Compare
* feat(plugins): Introduce Goyave as middleware * fix review from @System-Glitch * Add E2E tests to the CI * Fix config.json not found
* feat(plugins): Introduce Fiber as plugin * feat(plugins): Introduce Goyave as middleware (#203)
No description provided.