Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An idea: Make the body of log:* safe if a condition is signalled in its body. #47

Open
K1D77A opened this issue Jun 14, 2024 · 1 comment

Comments

@K1D77A
Copy link

K1D77A commented Jun 14, 2024

This is just an idea but this is a problem I have run into a few times and its more of a safe guard for programmer error than anything. But a common use case I have for log:* is to display a condition using the built in reporting ie (define-condition .. (:report ...)) but sometimes something like a slot might not be bound etc and a form like

(handler-case <something>
    (serious-condition (condition)
         (log:error "Something really went wrong: ~A" condition)
         <something else>
         ...

Will cause everything to go kaboom if there is a failure on the programmers part to fill the slots correctly in condition
A quick fix I have just tried is to wrap the actual printing of the condition into a macro that will catch any problems and return a different string but it seems fitting that log:* should have a fallback message in the case that the body of log:* signals a condition.

Does this already exist? If not how does it sound as a potential feature?

Thanks.

@scymtym
Copy link
Member

scymtym commented Aug 30, 2024

The current behavior has been explicitly designed by the original author with the intention of making it easy for users to debug problems in log statements. The main mechanism is

log4cl/src/logger.lisp

Lines 560 to 589 in fe3da51

(handler-bind
((error
(lambda (e)
;; if error is inside the actual user log statement
;; fall through, to invoke the debugger or such.
;;
;; Idea is that we want (log:info "~s" (something-causing-error))
;; to pop the debugger rather then go through appender error logic.
(unless *inside-user-log-function*
(case (if (eq orig-logger +self-meta-logger+) :ignore
(handle-appender-error appender e))
(:retry
(incf error-count)
(setf last-error e)
(cond ((< error-cnt 2)
(setf done nil))
;; manually disable if HANDLE-APPENDER-ERROR
;; can't fix it 3 times in a row
(t (setf enabled nil)
(log-appender-disabled appender e))))
(:ignore
(incf ignored-error-count)
(setf last-ignored-error e))
;; treat any other same as :disable
(t
(incf error-count)
(setf last-error e enabled nil)))
(return)))))
(progn (appender-do-append appender orig-logger level log-func)
(incf message-count))))
and
(deftest test-appender-error-in-user-log-statement-is-rethrown ()
"Test that when there is an condition is signaled from inside the
user log statement, its raised and does not disable the appender"
(with-package-log-hierarchy
(clear-logging-configuration)
(remove-all-appenders +self-logger+)
(let ((a1 (make-instance 'bad-appender)))
(with-slots (last-error count)
a1
(add-appender *root-logger* a1)
(log-config :d)
(log-info "hey")
(is (equal 0 (appender-error-count a1)))
(is (equal 1 count))
;; throws error doing append, which gets handled
(finishes (log-warn "hey"))
(is (equal 2 count))
(is (equal 1 (appender-error-count a1)))
(setf (appender-enabled-p a1) t
(appender-last-error a1) nil)
(log-info "hey")
(is (equal 1 (appender-error-count a1)))
(is (equal 3 count))
(signals error (log-info "~s" (function-with-error)))
(is (equal 4 count))
(is (null (appender-last-error a1)))
(is (equal 1 (appender-error-count a1)))))))
tests the behavior.

I can see benefits of the current behavior as well as of the behavior you are suggesting. It seems the best solution would be a configuration option or special variable to select one of the behaviors similar to the way the fiveam library provides fiveam:*on-error* to select a behavior for unexpected errors during test execution.

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

No branches or pull requests

2 participants