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

coro-http does not generate error nor returns when remote immediately closes the connection #217

Open
edubart opened this issue Jun 15, 2017 · 5 comments

Comments

@edubart
Copy link

edubart commented Jun 15, 2017

When doing a HTTP request with coro-http and the remote immediately closes the connection without a response the coroutine never returns, ideally it should throw an error.

Test exampe:

local http = require 'coro-http'
coroutine.wrap(function()
  local ok, err = pcall(function()
    http.request('GET', 'http://www.google.com:443/')
  end)
  if ok then print('success')
  else print('error:', err) end
end)()

In the above example because we are forcing connecting to port 443 (the https port) while using HTTP protocol the remote will imediatelly close the connection because of invalid protocol.

When running with luvit the above example ends with no messages, it should print the error instead.

@SinisterRectus
Copy link
Member

SinisterRectus commented Jul 30, 2017

There is a bit of a descrepency between how errors are handled between some of the coro packages. Some return nil, err, some throw the error. Changing this now may break things, though.

HTTP responses are a little different though since the code and reason are in the response metadata table. Depending upon the server, you may also get more information in the response body.

@creationix
Copy link
Member

I like the assert style error handling because it gives the caller the option to ignore errors without having to use error catching. Generally I only like to throw hard errors in cases of user error (invalid argument types, for example), but things that are outside the control of the programmer (like a website being down) are a different type of error and I like the soft error in returning nil, err.

@SinisterRectus
Copy link
Member

I agree, I prefer that style.

Some discrepancies I am referring to are:

@creationix
Copy link
Member

I'd be fine with PRs making it more consistent. We can bump major versions if it's a breaking change.

@SinisterRectus
Copy link
Member

I'll see what I can do then. It'll take some time to deconstruct everything, but it shouldn't be too challenging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants