bugfix: coroutine.wrap: propagate errors to the parent coroutine.

[thibaultcha]

I hereby granted the copyright of the changes in this pull request
to the authors of this lua-nginx-module project.

The manual states that coroutine.wrap is "Similar to the standard Lua API". However, the Lua API's coroutine.wrap has some nuances compared to the one provided by ngx_lua:

Returns the same values returned by resume, except the first boolean. In case of error, propagates the error.

Source (Lua 5.1 manual).

This caused me some issues in the daily job's latest release, where we expected errors encountered in coroutine.wrap to be propagated to the Lua main thread, and cause 500 errors. But those errors were swallowed by the thread "scheduler".

Example:

-- test.lua
local co = coroutine.wrap(function() return nnil.hello end)
print(co(1))
print("still running")

With LuaJIT 2.1:

$ luajit ./test.lua
luajit: /<path>/test.lua:2: /<path>/test.lua:1: attempt to index global 'nnil' (a nil value)
stack traceback:
	[C]: in function 'co'
	/<path>/test.lua:2: in main chunk
	[C]: at 0x0100000ef0

With our resty-cli utility:

$ resty ./test.lua
2018/01/23 14:50:25 [error] 20269#0: *2 lua coroutine: runtime error: /<path>/test.lua:1: attempt to index global 'nnil' (a nil value)
stack traceback:
coroutine 0:
	/<path>/test.lua: in function </<path>/test.lua:1>
coroutine 1:
	[C]: in function 'resume'
	coroutine.wrap:21: in function 'co'
	/<path>/test.lua:2: in function 'file_gen'
	init_worker_by_lua:43: in function <init_worker_by_lua:41>
	[C]: in function 'xpcall'
	init_worker_by_lua:50: in function <init_worker_by_lua:48>, context: ngx.timer
/<path>/test.lua:1: attempt to index global 'nnil' (a nil value)
still running

Notice the still running line at the bottom.

My initial fix for this was to edit the inline Lua code that creates coroutine.wrap so that it would propagate errors with error(..., 2), but it is a less than ideal fix, since the error becomes logged twice (once in the coroutine context, and once in the parent context), like so:

2018/01/23 15:01:35 [error] 49378#0: *2 lua coroutine: runtime error: content_by_lua(nginx.conf:49):4: something went wrong
stack traceback:
coroutine 0:
	[C]: in function 'error'
	content_by_lua(nginx.conf:49):4: in function <content_by_lua(nginx.conf:49):2>
coroutine 1:
	[C]: in function 'resume'
	coroutine.wrap:21: in function 'co'
	content_by_lua(nginx.conf:49):7: in function <content_by_lua(nginx.conf:49):1>, client: 127.0.0.1, server: localhost, request: "GET /t HTTP/1.1", host: "localhost"
2018/01/23 15:01:35 [error] 49378#0: *2 lua entry thread aborted: runtime error: content_by_lua(nginx.conf:49):7: content_by_lua(nginx.conf:49):4: something went wrong
stack traceback:
coroutine 0:
	[C]: in function 'error'
	coroutine.wrap:21: in function 'co'
	content_by_lua(nginx.conf:49):7: in function <content_by_lua(nginx.conf:49):1>, client: 127.0.0.1, server: localhost, request: "GET /t HTTP/1.1", host: "localhost"

Additionally, dealing with it in the Lua land would have meant a lot of expensive operations with the variadic arguments/return values aspects of coroutine.resume (we would have had to deal with pack, unpack, et al.).

Instead, I propose to get rid of the "hack" that is used to implement coroutine.wrap, and implement a version of it that embraces the ngx_lua threading model.

[thibaultcha]

@agentzh Do you have any feedback on this? Considering it is a behavior mistmatch according to both the Lua and ngx_lua manuals, I was under the impression that it would be of a somewhat middle to high priority?

[agentzh]

@thibaultcha This is a deep change. I'm not sure about it without thinking much longer about it.

[agentzh]

@thibaultcha Thank you for reporting this issue and contributing this patch!

[thibaultcha]

@agentzh Have you given more thought to this? Compatibility with Lua(JIT) is one of the goal we try to achieve with ngx_lua (as per previous incompatibilities we fixed) and that is why I believe this fix to be important. Thanks!

[agentzh]

@thibaultcha I understand but I have other higher priority things at the moment. Sorry. Will get back to this as soon as I can manage.