fix(plugin_iterator) guard against Lua error that happens inside plugin's "init_worker" handler

dndx · dndx · commit cc97625f6d62 · 2021-05-06T01:19:41.000+08:00
Previously when a plugin's "init_worker" handler fails, the rest of the
`init_worker` task are no longer executed. Although this is not
something that should happen in normal situation, there are certain
custom plugins that are not written correctly and failed "init_worker"
causes things like Hybrid mode to break because it's `init_worker` code
no longer executes.

This PR allows the Kong node to finish the rest of the initialization
even in case of plugin's "init_worker" handler error. It should improve
the situation because Kong core's initialization generally does not
depend on the outcome of a particular plugin's "init_worker" handler, and
this improves the stability of the product slightly.

Because failed "init_worker" handler is still considered a serious
event, we use the `stash_init_worker_error` function so the error
message will be printed for each incoming request later. This is the
same behavior as we do for the other initialization failures that could
occur in core.
diff --git a/kong/init.lua b/kong/init.lua
@@ -243,6 +243,7 @@ end
 local function execute_plugins_iterator(plugins_iterator, phase, ctx)
   local old_ws
   local delay_response
+  local errors
 
   if ctx then
     old_ws = ctx.workspace
@@ -262,7 +263,21 @@ local function execute_plugins_iterator(plugins_iterator, phase, ctx)
     kong_global.set_namespaced_log(kong, plugin.name)
 
     if not delay_response then
-      plugin.handler[phase](plugin.handler, configuration)
+      -- guard against failed handler in "init_worker" phase only because it will
+      -- cause Kong to not correctly initialize and can not be recovered automatically.
+      if phase == "init_worker" then
+        local ok, err = pcall(plugin.handler[phase], plugin.handler, configuration)
+        if not ok then
+          errors = errors or {}
+          errors[#errors + 1] = {
+            plugin = plugin.name,
+            err = err,
+          }
+        end
+
+      else
+        plugin.handler[phase](plugin.handler, configuration)
+      end
 
     elseif not ctx.delayed_response then
       local co = coroutine.create(plugin.handler.access)
@@ -282,6 +297,8 @@ local function execute_plugins_iterator(plugins_iterator, phase, ctx)
       ctx.workspace = old_ws
     end
   end
+
+  return errors
 end
 
 
@@ -616,7 +633,14 @@ function Kong.init_worker()
   end
 
   local plugins_iterator = runloop.get_plugins_iterator()
-  execute_plugins_iterator(plugins_iterator, "init_worker")
+  local errors = execute_plugins_iterator(plugins_iterator, "init_worker")
+  if errors then
+    for _, e in ipairs(errors) do
+      local err = "failed to execute the \"init_worker\" " ..
+                  "handler for plugin \"" .. e.plugin .."\": " .. e.err
+      stash_init_worker_error(err)
+    end
+  end
 
   runloop.init_worker.after()
 
diff --git a/spec/02-integration/05-proxy/04-plugins_triggering_spec.lua b/spec/02-integration/05-proxy/04-plugins_triggering_spec.lua
@@ -7,6 +7,7 @@ local pl_stringx = require "pl.stringx"
 
 
 local LOG_WAIT_TIMEOUT = 10
+local TEST_CONF = helpers.test_conf
 
 
 for _, strategy in helpers.each_strategy() do
@@ -1089,6 +1090,7 @@ for _, strategy in helpers.each_strategy() do
           assert(helpers.start_kong {
             database   = strategy,
             nginx_conf = "spec/fixtures/custom_nginx.template",
+            plugins = "short-circuit,init-worker-lua-error",
           })
 
           proxy_client = helpers.proxy_client()
@@ -1119,6 +1121,11 @@ for _, strategy in helpers.each_strategy() do
             message = "plugin executed"
           }, json)
         end)
+
+        it("protects against failed init_worker handler, FTI-2473", function()
+          local logs = pl_file.read(TEST_CONF.prefix .. "/" .. TEST_CONF.proxy_error_log)
+          assert.matches([[worker initialization error: failed to execute the "init_worker" handler for plugin "init-worker-lua-error"]], logs, nil, true)
+        end)
       end)
 
       if strategy ~= "off" then
diff --git a/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/handler.lua
@@ -0,0 +1,12 @@
+local InitWorkerLuaError = {}
+
+
+InitWorkerLuaError.PRIORITY = 1000
+
+
+function InitWorkerLuaError:init_worker(conf)
+  error("this fails intentionally")
+end
+
+
+return InitWorkerLuaError
diff --git a/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/schema.lua b/spec/fixtures/custom_plugins/kong/plugins/init-worker-lua-error/schema.lua
@@ -0,0 +1,10 @@
+return {
+  name = "init-worker-lua-error",
+  fields = {
+    { config = {
+        type = "record",
+        fields = { },
+      }
+    }
+  }
+}
