From f22fc2cc6ee42febb37c7c8b97d209f27edff0f3 Mon Sep 17 00:00:00 2001 From: Michal Cichra Date: Tue, 4 Apr 2017 11:07:05 +0200 Subject: [PATCH] [http] fix creating error responses they should always have access to the request object no matter what error it is --- CHANGELOG.md | 1 + .../src/resty/http_ng/backend/async_resty.lua | 32 ++++++++++---- apicast/src/resty/http_ng/backend/resty.lua | 2 +- apicast/src/resty/http_ng/response.lua | 4 +- .../http_ng/backend/async_resty_spec.lua | 44 +++++++++++++++++++ spec/resty/http_ng/backend/resty_spec.lua | 10 +++++ spec/resty/http_ng/response_spec.lua | 23 ++++++++++ 7 files changed, 105 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf12deda4..af40280db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Remove unnecessary code and comments [PR #344](https://github.com/3scale/apicast/pull/344) - JWT expiry not taken into account in authorization response cache [PR #283](https://github.com/3scale/apicast/pull/283) / [Issue #309](https://github.com/3scale/apicast/issues/309) / Fixed by [PR #341](https://github.com/3scale/apicast/pull/341) - Memory leak in round robin balancer [PR #345](https://github.com/3scale/apicast/pull/345) +- Error when trying to determine status of failed request when downloading configuration [PR #350](https://github.com/3scale/apicast/pull/350) ## [3.0.0-beta3] - 2017-03-20 diff --git a/apicast/src/resty/http_ng/backend/async_resty.lua b/apicast/src/resty/http_ng/backend/async_resty.lua index 1b380c18a..54cb7dcae 100644 --- a/apicast/src/resty/http_ng/backend/async_resty.lua +++ b/apicast/src/resty/http_ng/backend/async_resty.lua @@ -3,7 +3,6 @@ local rawset = rawset local pairs = pairs local unpack = unpack local assert = assert -local print = print local type = type local rawlen = rawlen local next = next @@ -18,18 +17,33 @@ local http = require 'resty.resolver.http' _M.async = function(request) local httpc = http.new() - local parsed_uri = assert(httpc:parse_uri(request.url)) + local parsed_uri = assert(httpc:parse_uri(assert(request.url, 'missing url'))) local scheme, host, port, path = unpack(parsed_uri) if not request.path then request.path = path end if #request.path == 0 then request.path = '/' end + local timeout = request.timeout or (request.options and request.options.timeout) + + if type(timeout) == 'number' then + httpc:set_timeout(timeout) + elseif type(timeout) == 'table' then + local connect_timeout = timeout.connect + local send_timeout = timeout.send + local read_timeout = timeout.read + + if httpc.set_timeouts then -- lua-resty-http >= 0.10 + httpc:set_timeouts(connect_timeout, send_timeout, read_timeout) + else + httpc.sock:settimeouts(connect_timeout, send_timeout, read_timeout) + end + end + local ok, err = httpc:connect(host, port) if not ok then - print('error connecting ', host, ':', port, ' : ', err) - return response.error(err, request) + return response.error(request, err) end if scheme == 'https' then @@ -40,7 +54,7 @@ _M.async = function(request) session, err = httpc:ssl_handshake(false, host, verify) if not session then - return response.error(err, request) + return response.error(request, err) end end @@ -50,11 +64,11 @@ _M.async = function(request) if res then return response.new(request, res.status, res.headers, function() return (res:read_body()) end) else - return response.error(err, request) + return response.error(request, err) end end -local function future(thread) +local function future(thread, request) local ok, res local function load(table) @@ -63,7 +77,7 @@ local function future(thread) rawset(table, 'ok', ok) - if not ok then res = response.error(res) end + if not ok then res = response.error(request, res or 'failed to create async request') end for k,v in pairs(res) do if k == 'headers' then @@ -94,7 +108,7 @@ end _M.send = function(request) local thread = spawn(_M.async, request) - return future(thread) + return future(thread, request) end return _M diff --git a/apicast/src/resty/http_ng/backend/resty.lua b/apicast/src/resty/http_ng/backend/resty.lua index 8f6fefcd5..15b5e6174 100644 --- a/apicast/src/resty/http_ng/backend/resty.lua +++ b/apicast/src/resty/http_ng/backend/resty.lua @@ -24,7 +24,7 @@ backend.send = function(request) if res then return response.new(request, res.status, res.headers, res.body) else - return response.error(err) + return response.error(request, err) end end diff --git a/apicast/src/resty/http_ng/response.lua b/apicast/src/resty/http_ng/response.lua index fc931bda8..d2e7f275e 100644 --- a/apicast/src/resty/http_ng/response.lua +++ b/apicast/src/resty/http_ng/response.lua @@ -50,7 +50,9 @@ function response.new(request, status, headers, body) return setmetatable(res, mt) end -function response.error(message, request) +function response.error(request, message) + assert(request, 'missing request') + assert(message, 'missing message') return { ok = false, error = message, request = request, status = 0, headers = {} } end diff --git a/spec/resty/http_ng/backend/async_resty_spec.lua b/spec/resty/http_ng/backend/async_resty_spec.lua index 033650621..d66281b56 100755 --- a/spec/resty/http_ng/backend/async_resty_spec.lua +++ b/spec/resty/http_ng/backend/async_resty_spec.lua @@ -29,6 +29,50 @@ describe('resty backend', function() assert.equal('string', type(response.body)) assert.truthy(response.request) end) + + it('returns proper error on connect timeout', function() + local req = { method = method, url = 'http://example.com:81/', timeout = { connect = 1 } } + + local response = backend.send(req) + + assert.truthy(response) + assert.equal('timeout', response.error) + assert.equal(req, response.request) + assert.falsy(response.ok) + end) + + it('returns proper error on read timeout', function() + local req = { method = method, url = 'http://example.com/', timeout = { read = 1 } } + + local response = backend.send(req) + + assert.truthy(response) + assert.equal('timeout', response.error) + assert.equal(req, response.request) + assert.falsy(response.ok) + end) + + it('returns proper error on invalid ssl', function() + local req = { method = method, url = 'https://untrusted-root.badssl.com/', options = { ssl = { verify = true } } } + + local response = backend.send(req) + + assert.truthy(response) + assert.match('unable to get local issuer certificate', response.error) + assert.equal(req, response.request) + assert.falsy(response.ok) + end) + + it('returns proper error on invalid request', function() + local req = { method = method } + + local response = backend.send(req) + + assert.truthy(response) + assert.match('failed to create async request', response.error) + assert.equal(req, response.request) + assert.falsy(response.ok) + end) end) describe('when there is no error', function() diff --git a/spec/resty/http_ng/backend/resty_spec.lua b/spec/resty/http_ng/backend/resty_spec.lua index ebcfd2504..c955e230e 100755 --- a/spec/resty/http_ng/backend/resty_spec.lua +++ b/spec/resty/http_ng/backend/resty_spec.lua @@ -26,5 +26,15 @@ describe('resty backend', function() assert.truthy(response.request) assert(response.headers.location:match('^https://')) end) + + it('returns error', function() + local req = { method = method, url = 'http://0.0.0.0:0/' } + local response, err = backend.send(req) + + assert.falsy(err) + assert.truthy(response.error) + assert.falsy(response.ok) + assert.same(req, response.request) + end) end) end) diff --git a/spec/resty/http_ng/response_spec.lua b/spec/resty/http_ng/response_spec.lua index e69de29bb..77dd606e9 100755 --- a/spec/resty/http_ng/response_spec.lua +++ b/spec/resty/http_ng/response_spec.lua @@ -0,0 +1,23 @@ +local _M = require 'resty.http_ng.response' +local request = require 'resty.http_ng.request' + +describe('http_ng response', function() + + describe('error', function() + + local req = request.new{ method = 'GET', url = 'http://example.com' } + + it('has the request', function() + local error = _M.error(req, 'failed') + + assert.equal(req, error.request) + end) + + + it('has the message', function() + local error = _M.error(req, 'error message') + + assert.equal('error message', error.error) + end) + end) +end)