From eb1be84f793b8dfb1acdb1103e6c1935d5290972 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Wed, 8 Sep 2021 20:09:52 +0300 Subject: [PATCH 1/3] Throw an error when http_middelware is processing a wrong handler --- metrics/http_middleware.lua | 3 +++ test/http_middleware_test.lua | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/metrics/http_middleware.lua b/metrics/http_middleware.lua index f379a036..a32f5c1e 100644 --- a/metrics/http_middleware.lua +++ b/metrics/http_middleware.lua @@ -71,6 +71,9 @@ end -- ... arguments for pcall to instrument function export.observe(collector, route, ...) return collector:observe_latency(function(ok, result) + if result == nil then + error("incorrect http handler", 0) + end return { path = route.path, method = route.method, diff --git a/test/http_middleware_test.lua b/test/http_middleware_test.lua index 97ff6ba8..31323d7d 100644 --- a/test/http_middleware_test.lua +++ b/test/http_middleware_test.lua @@ -148,6 +148,17 @@ g.test_v1_middleware = function() ) end +g.test_v1_wrong_handler = function() + local request = {endpoint = table.copy(route)} + + local observer = stub_observer(function() end) + local handler = function() + -- we forget to return result! + -- return result + end + t.assert_error_msg_contains('incorrect http handler', http_middleware.v1(handler, observer), request) +end + g.test_v2_middleware = function() local httpd = require('http.server').new('127.0.0.1', 12345) t.skip_if(httpd.set_router == nil, 'Skip http 2.x test') @@ -170,3 +181,21 @@ g.test_v2_middleware = function() 'number' ) end + +g.test_v2_wrong_handler = function() + local httpd = require('http.server').new('127.0.0.1', 12345) + t.skip_if(httpd.set_router == nil, 'Skip http 2.x test') + local router = require('http.router').new() + router:route(route, function() + -- we forget to return result! + -- return result + end) + router:use(http_middleware.v2(), {name = 'http_instrumentation'}) + httpd:set_router(router) + httpd:start() + local client = require('http.client').new() + local response = client:request(route.method, 'http://127.0.0.1:12345' .. route.path) + httpd:stop() + t.assert_equals(response.status, 500) + t.assert_str_contains(response.body, 'incorrect http handler') +end From bc5102e7d95b7b99608f6280c91d11696114d7d8 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Wed, 8 Sep 2021 20:12:11 +0300 Subject: [PATCH 2/3] CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa4defdc..b91d0214 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Removed - Average collector +### Fixed +- Throw an error when http_middelware is processing a wrong handler [#199](https://github.com/tarantool/metrics/issues/199) + ## [0.10.0] - 2021-08-03 ### Changed - metrics registry refactoring to search with `O(1)` [#188](https://github.com/tarantool/metrics/issues/188) From ea13c0547cabb8e8aadde579248229863477ae39 Mon Sep 17 00:00:00 2001 From: Igor Zolotarev Date: Thu, 9 Sep 2021 16:00:44 +0300 Subject: [PATCH 3/3] Change error message --- metrics/http_middleware.lua | 5 +++-- test/http_middleware_test.lua | 10 ++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/metrics/http_middleware.lua b/metrics/http_middleware.lua index a32f5c1e..ce6a629e 100644 --- a/metrics/http_middleware.lua +++ b/metrics/http_middleware.lua @@ -71,8 +71,9 @@ end -- ... arguments for pcall to instrument function export.observe(collector, route, ...) return collector:observe_latency(function(ok, result) - if result == nil then - error("incorrect http handler", 0) + if type(result) ~= 'table' then + error(('incorrect http handler for %s %s: expecting return response object'): + format(route.method, route.path), 0) end return { path = route.path, diff --git a/test/http_middleware_test.lua b/test/http_middleware_test.lua index 31323d7d..e4dfcacc 100644 --- a/test/http_middleware_test.lua +++ b/test/http_middleware_test.lua @@ -156,7 +156,10 @@ g.test_v1_wrong_handler = function() -- we forget to return result! -- return result end - t.assert_error_msg_contains('incorrect http handler', http_middleware.v1(handler, observer), request) + t.assert_error_msg_contains( + 'incorrect http handler for POST /some/path: expecting return response object', + http_middleware.v1(handler, observer), request + ) end g.test_v2_middleware = function() @@ -197,5 +200,8 @@ g.test_v2_wrong_handler = function() local response = client:request(route.method, 'http://127.0.0.1:12345' .. route.path) httpd:stop() t.assert_equals(response.status, 500) - t.assert_str_contains(response.body, 'incorrect http handler') + t.assert_str_contains( + response.body, + 'incorrect http handler for POST /some/path: expecting return response object' + ) end