http/server: close connection on unhandled internal errors #148
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Also, I have no idea how to write for this because it would require a way to cause exception from client side ... which is certainly something I do not want to see in server code ;-) |
daurnimator
requested changes
Apr 12, 2019
Owner
daurnimator
left a comment
daurnimator
left a comment
There was a problem hiding this comment.
I haven't had time to review properly. Just things that immediately jumped out at me.
http/server.lua
Outdated
| stream, err, errno = conn:get_next_incoming_stream(timeout) | ||
| local stream_or_err | ||
| local finished | ||
| finished, stream_or_err, err, errno = xpcall(conn.get_next_incoming_stream, function(err) return debug.traceback(err) end, conn, timeout) |
Owner
There was a problem hiding this comment.
try and avoid xpcall on hot paths; it has quite an impact.
Also you can pass debug.traceback directly.
Contributor
Author
There was a problem hiding this comment.
I'm not Lua expert - can you advise a different approach to catch errors instead of xpcall?
Contributor
Author
There was a problem hiding this comment.
I wonder if it would be sufficient to wrap whole while cycle in xpcall(). Opinions?
Fixes: daurnimator#146 I'm not sure if implementation is correct, or if it best way to handle it. In any case it fixes client-visible problem where lua-http server keep connection open even after internal error (like daurnimator#145).
pspacek
force-pushed
the
throw_kill_connection
branch
from
026f9af to
d254321
Compare
1 similar comment
Owner
|
See #146 (comment) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes: #146
I'm not sure if implementation is correct, or if it best way to handle
it. In any case it fixes client-visible problem where lua-http server
keep connection open even after internal error (like #145).