Document adapter error handling requirements and add propagation tests

README.md

@@ -162,6 +162,38 @@ We also provide a [web-based UI](https://casbin.org/docs/admin-portal) for model
 
 https://casbin.org/docs/adapters
 
+### Error Handling for Adapters
+
+When implementing or using custom adapters (e.g., database adapters for PostgreSQL, MySQL, etc.), it's important to ensure proper error handling:
+
+**For Adapter Developers:**
+- Adapters MUST raise errors using `error()` when operations fail (e.g., database connection failures, authentication errors).
+- Do NOT silently log errors without raising them, as this prevents users from catching errors with `pcall()`.
+- Example of correct error handling:
+```lua
+function MyAdapter:loadPolicy(model)
+    local conn, err = connect_to_database(self.config)
+    if not conn then
+        error("Database connection failed: " .. tostring(err))
+    end
+    -- ... continue with loading policy
+end
+```
+
+**For Adapter Users:**
+- You can catch errors from adapter operations using `pcall()`:
+```lua
+local ok, err = pcall(function()
+    local adapter = MyAdapter:new(config)
+    local enforcer = Enforcer:new("model.conf", adapter)
+end)
+
+if not ok then
+    print("Error loading policy: " .. tostring(err))
+end
+```
+- If errors appear in logs but `pcall()` doesn't catch them, the adapter may not be raising errors properly. Contact the adapter maintainer to fix this.
+
 ## Policy consistence between multiple nodes
 
 https://casbin.org/docs/watchers
@@ -194,6 +226,22 @@ Authz middlewares for web frameworks: https://casbin.org/docs/middlewares
 
 https://casbin.org/docs/adopters
 
+## Troubleshooting
+
+### Errors not caught by pcall()
+
+**Problem:** When using an adapter (e.g., PostgreSQL adapter) with incorrect configuration (wrong password, etc.), errors appear in `error.log` but cannot be caught with `pcall()`.
+
+**Cause:** The adapter is logging errors (e.g., using `ngx.log()` in OpenResty) instead of raising them with `error()`.
+
+**Solution:**
+1. Verify the adapter is implementing the Casbin adapter interface correctly - it should raise errors using `error()` when operations fail
+2. If you're using a third-party adapter, report this issue to the adapter maintainer
+3. As a workaround, check logs for errors and handle them at the application level
+4. Consider switching to an adapter that properly raises errors
+
+For adapter developers: See the [Policy persistence](#policy-persistence) section for requirements on proper error handling.
+
 ## How to Contribute
 
 Please read the [contributing guide](CONTRIBUTING.md).

src/persist/Adapter.lua

@@ -52,6 +52,9 @@ end
         * loadPolicy loads all policy rules from the storage.
         *
         * @param model the model.
+        * @raises error if the policy cannot be loaded (e.g., database connection failure).
+        *         Implementations MUST raise an error using error() on failure so that
+        *         callers can catch it with pcall().
 ]]
 function Adapter:loadPolicy(model)
 
@@ -61,6 +64,9 @@ end
      * savePolicy saves all policy rules to the storage.
      *
      * @param model the model.
+     * @raises error if the policy cannot be saved (e.g., database connection failure).
+     *         Implementations MUST raise an error using error() on failure so that
+     *         callers can catch it with pcall().
 ]]
 function Adapter:savePolicy(model)
 
@@ -73,6 +79,9 @@ end
      * @param sec the section, "p" or "g".
      * @param ptype the policy type, "p", "p2", .. or "g", "g2", ..
      * @param rule the rule, like (sub, obj, act).
+     * @raises error if the policy cannot be added (e.g., database connection failure).
+     *         Implementations MUST raise an error using error() on failure so that
+     *         callers can catch it with pcall().
 ]]
 function Adapter:addPolicy(sec, ptype, rule)
 
@@ -85,6 +94,9 @@ end
      * @param sec the section, "p" or "g".
      * @param ptype the policy type, "p", "p2", .. or "g", "g2", ..
      * @param rule the rule, like (sub, obj, act).
+     * @raises error if the policy cannot be removed (e.g., database connection failure).
+     *         Implementations MUST raise an error using error() on failure so that
+     *         callers can catch it with pcall().
 ]]
 function Adapter:removePolicy(sec, ptype, rule)
 
@@ -99,6 +111,9 @@ end
      * @param fieldIndex the policy rule's start index to be matched.
      * @param fieldValues the field values to be matched, value ""
      *                    means not to match this field.
+     * @raises error if the policy cannot be removed (e.g., database connection failure).
+     *         Implementations MUST raise an error using error() on failure so that
+     *         callers can catch it with pcall().
 ]]
 function Adapter:removeFilteredPolicy(sec, ptype, fieldIndex, fieldValues)
 

src/persist/FilteredAdapter.lua

@@ -24,6 +24,9 @@ setmetatable(FilteredAdapter, Adapter)
     * loadFilteredPolicy loads only policy rules that match the filter.
     * @param model the model.
     * @param filter the filter used to specify which type of policy should be loaded.
+    * @raises error if the policy cannot be loaded (e.g., database connection failure).
+    *         Implementations MUST raise an error using error() on failure so that
+    *         callers can catch it with pcall().
 ]]
 function FilteredAdapter:loadFilteredPolicy(model, filter)
 

tests/persist/error_handling_spec.lua

@@ -0,0 +1,132 @@
+--Copyright 2021 The casbin Authors. All Rights Reserved.
+--
+--Licensed under the Apache License, Version 2.0 (the "License");
+--you may not use this file except in compliance with the License.
+--You may obtain a copy of the License at
+--
+--    http://www.apache.org/licenses/LICENSE-2.0
+--
+--Unless required by applicable law or agreed to in writing, software
+--distributed under the License is distributed on an "AS IS" BASIS,
+--WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+--See the License for the specific language governing permissions and
+--limitations under the License.
+
+local Enforcer = require("src.main.Enforcer")
+local Adapter = require("src.persist.Adapter")
+local path = os.getenv("PWD") or io.popen("cd"):read()
+
+describe("Error handling tests", function ()
+    it("should catch errors from adapter during enforcer initialization", function ()
+        -- Create a failing adapter that throws an error during loadPolicy
+        local FailingAdapter = {}
+        setmetatable(FailingAdapter, Adapter)
+        FailingAdapter.__index = FailingAdapter
+
+        function FailingAdapter:new()
+            local o = {}
+            setmetatable(o, self)
+            self.__index = self
+            return o
+        end
+
+        function FailingAdapter:loadPolicy(model)
+            error("Database connection failed: authentication failed")
+        end
+
+        local model = path .. "/examples/rbac_model.conf"
+        local a = FailingAdapter:new()
+
+        -- Test that the error can be caught with pcall and has the expected message
+        local ok, err = pcall(function()
+            local e = Enforcer:new(model, a)
+        end)
+
+        assert.is.False(ok)
+        assert.is.truthy(string.find(err, "Database connection failed"))
+
+        -- Also verify using assert.has_error pattern
+        assert.has_error(function()
+            local a2 = FailingAdapter:new()
+            local e2 = Enforcer:new(model, a2)
+        end, "Database connection failed")
+    end)
+
+    it("should catch errors from adapter during explicit loadPolicy call", function ()
+        -- Create an adapter that succeeds initially but fails on explicit loadPolicy
+        local DelayedFailingAdapter = {}
+        setmetatable(DelayedFailingAdapter, Adapter)
+        DelayedFailingAdapter.__index = DelayedFailingAdapter
+
+        function DelayedFailingAdapter:new()
+            local o = {}
+            setmetatable(o, self)
+            self.__index = self
+            -- Set isFiltered = true to prevent automatic loadPolicy during Enforcer initialization.
+            -- This allows us to test explicit loadPolicy() calls separately.
+            o.isFiltered = true
+            return o
+        end
+
+        function DelayedFailingAdapter:loadPolicy(model)
+            error("Database connection failed: wrong password")
+        end
+
+        local model = path .. "/examples/rbac_model.conf"
+        local a = DelayedFailingAdapter:new()
+
+        -- Create enforcer (won't load policy due to isFiltered=true)
+        local e = Enforcer:new(model, a)
+
+        -- Test that calling loadPolicy explicitly raises an error that can be caught
+        local ok, err = pcall(function()
+            e:loadPolicy()
+        end)
+
+        assert.is.False(ok)
+        assert.is.truthy(string.find(err, "Database connection failed"))
+
+        -- Also verify using assert.has_error pattern
+        assert.has_error(function()
+            e:loadPolicy()
+        end, "wrong password")
+    end)
+
+    it("should catch errors from adapter during loadFilteredPolicy call", function ()
+        -- Create an adapter that fails during loadFilteredPolicy
+        local FilteredFailingAdapter = {}
+        setmetatable(FilteredFailingAdapter, Adapter)
+        FilteredFailingAdapter.__index = FilteredFailingAdapter
+
+        function FilteredFailingAdapter:new()
+            local o = {}
+            setmetatable(o, self)
+            self.__index = self
+            -- Set isFiltered = true to prevent automatic loadPolicy during Enforcer initialization.
+            -- This allows us to test loadFilteredPolicy() calls independently.
+            o.isFiltered = true
+            return o
+        end
+
+        function FilteredFailingAdapter:loadFilteredPolicy(model, filter)
+            error("Database query failed: connection timeout")
+        end
+
+        local model = path .. "/examples/rbac_model.conf"
+        local a = FilteredFailingAdapter:new()
+        local e = Enforcer:new(model, a)
+
+        -- Test that error from loadFilteredPolicy can be caught
+        local ok, err = pcall(function()
+            e:loadFilteredPolicy({})
+        end)
+
+        assert.is.False(ok)
+        assert.is.truthy(string.find(err, "Database query failed"))
+
+        -- Also verify using assert.has_error pattern
+        assert.has_error(function()
+            e:loadFilteredPolicy({})
+        end, "connection timeout")
+    end)
+end)