fix(balancer) do not send body from balancer ctx

[thibaultcha]

Summary

The kong.tools.responses module sends a body with ngx.say when
calling responses.send_HTTP_SERVER_ERROR(). This produces an error on
top of a previous error, as in:

2017/04/03 23:42:00 [error] 11076#0: *29 [lua] kong.lua:239: balancer(): failed to set the current peer (address:'2400:cb00:2048:1:0:0:681c:767' port:80): invalid port while connecting to upstream, client: 127.0.0.1, server: kong, request: "GET /status/200?apikey=secret123 HTTP/1.1", host: "acl_test1.com"
2017/04/03 23:42:00 [error] 11076#0: *29 failed to run balancer_by_lua*: ...he/luarocks-2.4.2/share/lua/5.1/kong/tools/responses.lua:127: API disabled in the context of balancer_by_lua*
stack traceback:
	[C]: in function 'say'
	...he/luarocks-2.4.2/share/lua/5.1/kong/tools/responses.lua:127: in function 'balancer'
	balancer_by_lua:2: in function <balancer_by_lua:1> while connecting to upstream, client: 127.0.0.1, server: kong, request: "GET /status/200?apikey=secret123 HTTP/1.1", host: "acl_test1.com"'

This is because, as pointed out in the error, ngx.say is not supported
in the balancer_by_lua context.

This logs the errors directly from the balancer context and simply sends
the 500 status to the client.

[Tieske]

@thibaultcha I'll have a look tomorrow, first thought: wouldn't the generic solution be to test in responses whether say is supported and handle it there generically?

[thibaultcha]

Not so sure, since this module is bound to disappear imo. With time, I regret ever implementing it, since I'd much rather favor explicitness and avoid black boxes for the core. Plugins can use such a module (rewritten as part of the Public Lua API, this one should be considered legacy), but I believe the core should not and should use the raw OR Lua API.

[Tieske]

I don't agree to that. That module has some logic (like 500's not leaking any info, and default messages). Also this logic, not using say in unsupported contexts, should be there imo. If you do not centralize that code, you'll have duplicate code all over the place, and lots of bugs regarding leaking info, non standard messages, and accidental say usage anyway.

It would be asking for trouble imo.

[Tieske]

Considering i18n and custom error messages pending, we should probably postpone this design decision until we actually get to refactoring responses. And in that case, we should stick to responses for now.

[thibaultcha]

I don't mean duplicating it all over the place, but rather do less "black box" things that we have no control of from the caller's POV and that is taking the proper OR API we want to manipulate out of its reach (setting headers, arbitrary logging, arbitrary content-type setting or body encoding, arbitrary exit code, etc, etc...).

BTW, it might be worth noting that this patch still uses the send() method, and hence, the responses module. It is also worth noting that prior to this patch, one of those hunks was not making use of the logging error feature from the responses module in the first place. It is also worth noting that using the responses module forces us to do Lua-land string concatenation.

I am not opposed to making the responses module more resistant to such usage, but then the scope of this PR changes and it should be done in a separate one, with a bigger patch. Aka: not using OR APIs when they are not supposed to be used. This covers: ngx.status, ngx.header, ngx.say, and ngx.exit. Will you contribute such a patch in a separate PR?