From 2a190736a58674086c3a27bf71a7993383fffb55 Mon Sep 17 00:00:00 2001
From: Thibault Charbonnier <thibaultcha@me.com>
Date: Thu, 8 Aug 2019 15:32:45 -0700
Subject: [PATCH] change: removed error logging on coroutine runtime errors.

---
 src/ngx_http_lua_util.c | 44 +++++++++++++++++++++++++--------------
 t/091-coroutine.t       | 46 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/src/ngx_http_lua_util.c b/src/ngx_http_lua_util.c
index c7ed643a0c..5cd5bac5d8 100644
--- a/src/ngx_http_lua_util.c
+++ b/src/ngx_http_lua_util.c
@@ -1020,6 +1020,14 @@ ngx_http_lua_run_thread(lua_State *L, ngx_http_request_t *r,
 
     NGX_LUA_EXCEPTION_TRY {
 
+        /*
+         * silence a -Werror=clobbered warning with gcc 5.4
+         * due to above setjmp
+         */
+        err = NULL;
+        msg = NULL;
+        trace = NULL;
+
         if (ctx->cur_co_ctx->thread_spawn_yielded) {
             ngx_http_lua_probe_info("thread spawn yielded");
 
@@ -1389,29 +1397,33 @@ ngx_http_lua_run_thread(lua_State *L, ngx_http_request_t *r,
                 ctx->cur_co_ctx = orig_coctx;
             }
 
-            if (lua_isstring(ctx->cur_co_ctx->co, -1)) {
-                dd("user custom error msg");
-                msg = lua_tostring(ctx->cur_co_ctx->co, -1);
-
-            } else {
-                msg = "unknown reason";
-            }
-
             ngx_http_lua_cleanup_pending_operation(ctx->cur_co_ctx);
 
             ngx_http_lua_probe_coroutine_done(r, ctx->cur_co_ctx->co, 0);
 
             ctx->cur_co_ctx->co_status = NGX_HTTP_LUA_CO_DEAD;
 
-            ngx_http_lua_thread_traceback(L, ctx->cur_co_ctx->co,
-                                          ctx->cur_co_ctx);
-            trace = lua_tostring(L, -1);
+            if (orig_coctx->is_uthread
+                || orig_coctx->is_wrap
+                || ngx_http_lua_is_entry_thread(ctx))
+            {
+                ngx_http_lua_thread_traceback(L, orig_coctx->co, orig_coctx);
+                trace = lua_tostring(L, -1);
+
+                if (lua_isstring(orig_coctx->co, -1)) {
+                    msg = lua_tostring(orig_coctx->co, -1);
+                    dd("user custom error msg: %s", msg);
 
-propagate_error:
+                } else {
+                    msg = "unknown reason";
+                }
+            }
 
-            ngx_http_lua_assert(err != NULL && msg != NULL && trace != NULL);
+propagate_error:
 
             if (ctx->cur_co_ctx->is_uthread) {
+                ngx_http_lua_assert(err != NULL && msg != NULL
+                                    && trace != NULL);
 
                 ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
                               "lua user thread aborted: %s: %s\n%s",
@@ -1462,6 +1474,9 @@ ngx_http_lua_run_thread(lua_State *L, ngx_http_request_t *r,
             }
 
             if (ngx_http_lua_is_entry_thread(ctx)) {
+                ngx_http_lua_assert(err != NULL && msg != NULL
+                                    && trace != NULL);
+
                 ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
                               "lua entry thread aborted: %s: %s\n%s",
                               err, msg, trace);
@@ -1519,9 +1534,6 @@ ngx_http_lua_run_thread(lua_State *L, ngx_http_request_t *r,
             lua_xmove(orig_coctx->co, next_co, 1);
             nrets = 2;
 
-            ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
-                          "lua coroutine: %s: %s\n%s", err, msg, trace);
-
             /* try resuming on the new coroutine again */
             continue;
         }
diff --git a/t/091-coroutine.t b/t/091-coroutine.t
index c9ee72c7c4..5af1ea3a19 100644
--- a/t/091-coroutine.t
+++ b/t/091-coroutine.t
@@ -467,21 +467,25 @@ done
 
 
 === TEST 10: thread traceback (multi-thread)
+Note: only coroutine.wrap propagates errors to the parent coroutine
+(and thus produces a traceback)
 --- config
     location /lua {
-        content_by_lua '
+        content_by_lua_block {
             local f = function(cr) coroutine.resume(cr) end
             -- emit a error
             local g = function() unknown.unknown = 1 end
-            local l1 = coroutine.create(f)
-            local l2 = coroutine.create(g)
-            coroutine.resume(l1, l2)
+            local l1 = coroutine.wrap(f)
+            local l2 = coroutine.wrap(g)
+            local l3 = coroutine.wrap(function() l1(l2) end)
+            l3()
             ngx.say("hello")
-        ';
+        }
     }
 --- request
 GET /lua
---- response_body
+--- error_code: 500
+--- response_body_unlike
 hello
 --- error_log eval
 ["stack traceback:", "coroutine 0:", "coroutine 1:", "coroutine 2:"]
@@ -904,8 +908,8 @@ qr/^child: resume: falsecontent_by_lua\(nginx\.conf:\d+\):4: bad
 child: status: dead
 parent: status: running
 $/s
---- error_log eval
-qr/lua coroutine: runtime error: content_by_lua\(nginx\.conf:\d+\):4: bad/
+--- no_error_log
+[error]
 
 
 
@@ -1712,3 +1716,29 @@ GET /t
 --- grep_error_log eval: qr/init_by_lua error: .*? something went wrong/
 --- grep_error_log_out
 init_by_lua error: init_by_lua:7: init_by_lua:4: something went wrong
+
+
+
+=== TEST 46: coroutine.resume runtime errors do not log errors
+--- config
+    location = /t {
+        content_by_lua_block {
+            local function f()
+                error("something went wrong")
+            end
+
+            local ret1, ret2 = coroutine.resume(coroutine.create(f))
+            ngx.say(ret1)
+            ngx.say(ret2)
+        }
+    }
+--- request
+GET /t
+--- response_body_like
+false
+content_by_lua\(nginx.conf:\d+\):\d+: something went wrong
+--- no_error_log eval
+[
+    qr/\[error\] .*? lua coroutine: runtime error:",
+    "stack traceback:",
+]