-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
http/server: close connection on unhandled internal errors #148
Conversation
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 ;-) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try and avoid xpcall
on hot paths; it has quite an impact.
Also you can pass debug.traceback
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not Lua expert - can you advise a different approach to catch errors instead of xpcall
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
026f9af
to
d254321
Compare
1 similar comment
See #146 (comment) |
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).