Skip to content
This repository has been archived by the owner on May 31, 2022. It is now read-only.

Auditing failures shouldn't cause functional failures #16

Open
apetro opened this issue Feb 25, 2013 · 5 comments
Open

Auditing failures shouldn't cause functional failures #16

apetro opened this issue Feb 25, 2013 · 5 comments

Comments

@apetro
Copy link

apetro commented Feb 25, 2013

Currently, Inspektr asserts that its arguments not be null. When these assertions fail, the action being audited fails.

Instead, failure in the audit trail system should not induce failure in the system being audited. Failure should be logged and control return from the Aspect to the functional code wrapped.

Cf. Issue #13 , which helped but didn't go far enough.

Cf. https://groups.google.com/d/topic/cas-user/e2FRrJj0BFU/discussion

@battags
Copy link
Collaborator

battags commented Feb 25, 2013

Having read the discussion where root cause has still not been determined, it would be good to confirm what the actual issue is before creating this issue. The issue could lie elsewhere.

@battags
Copy link
Collaborator

battags commented Feb 25, 2013

Also, at worst failure should be optional. There are numerous environments where one should not be allowed to proceed if something cannot be audited.

@serac
Copy link
Contributor

serac commented Feb 25, 2013

#10 is in the same vein, although perhaps more specific, as this issue. Some extra error handling is harmless at worst and incredibly valuable at best. I would think you would at least want to consider it.

@sunnymoon
Copy link

We had this problem in production system... With JASIG CAS server. One error during an @Audit method caused the AuditTrailManagementAspect to catch that original exception, which it should rethrow (and normally would), but as one of the resolvers couldn't actually obtain the correct information, the "executeAuditCode" method blew up with an exception and the original exception was totally forgotten.

I guess that in the methods "handleAuditTrail", we could change it to something more solid in terms of exception reporting.

@sunnymoon
Copy link

I just created a pull request with a proposed solution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants