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

Cleanup: logging methods #880

Closed
wants to merge 3 commits into from
Closed

Cleanup: logging methods #880

wants to merge 3 commits into from

Conversation

veryrusty
Copy link
Member

The DSL keywords for logging (debug, info, warning and error) as well as Role::Hookable now call the log method on an app object.

While this doesn't solve #791, it reduces the number of cases we need to deal with for the "magic" number of call stacks to skip to report the caller from 4 to 2.

Avoid a level of indirection my making the DSL logging keywords call
$app->log directly, rather than calling the DSL 'log' keyword.

This is a cleanup to help resolve the "magic number" of call frames to
jump in the logger for reporting what triggered the log message. i.e.
the `log` and `debug`, `info`, `warning` and `error` each need to jump
the same nunmber of call frames to report what triggered the log
message.
Generate log entries on hook execution by calling $app->log(), rather
than the logging engine directly. This helps minimize the number of
callers involved in generating log messages, which helps minimize the
magic number of call frames to skip when reporting the caller in log
messages.

This approach also provides a single point to deal with future handling of
log messages when no logging engine is defined for an app.
Now the DSL logging keywords call $app->log() directly, we need to skip
one less call frame when reporting the caller.

Updates the concole logger tests, as the 5th stack frame exists - and is
the test file, thanks to Capture::Tiny adding the exact same number of
frames in capturing the output.
@xsawyerx
Copy link
Member

👍

@xsawyerx xsawyerx closed this Apr 19, 2015
@xsawyerx xsawyerx deleted the cleanup/logging_methods branch April 19, 2015 16:47
@xsawyerx
Copy link
Member

Merged, thanks! 👍

xsawyerx added a commit that referenced this pull request Apr 26, 2015
    [ BUG FIXES ]
    * GH #868: Fix incorrect access name in $error->throw. (cdmalon)
    * GH #879, #883: Fix version numbering in packaging and tests.
      (Russell Jenkins)
    * File serving (send_file) won't call serializer. (Russell Jenkins)
    * GH #892, #510: Workaround for multiple plugins with hooks.
      (Russell Jenkins, Alberto Simões)
    * GH #558: Remove "prefix" inconsistency with possibly missing postfixed
      forward slash. (Sawyer X)

    [ DOCUMENTATION ]
    * GH #816, #874 Document session engine changes in migration documentation.
      (Chenchen Zhao)
    * GH #866, #870: Clarify that you cannot forward to a static file, why,
      and two different ways of accomplishing it without forward.
      (Sakshee Vijayvargia)
    * GH #878: Rework example for optional named matching due to operator
      precedence. (Andrew Solomon)
    * GH #844: Document Simple session backend is the default. (Sawyer X)

    [ ENHANCEMENT ]
    * GH #869: Streaming file serving (send_file). (Russell Jenkins)
    * GH #793: "prefix" now supports the path definition spec. (Sawyer X)
    * GH #817, #845: Route spec under a prefix doesn't need to start with
      a slash (but must without a prefix).
      (Sawyer X, Russell Jenkins)
    * GH #871: Use Safe.pm instead of eval with Dancer2::Serializer::Dumper.
      (David Zurborg)
    * GH #880: Reduce and cleanup different logging calls in order to handle
      the stack frames traceback for logging classes. (Russell Jenkins)
    * GH #857, #875: When failing to render in Template::Toolkit, make the
      error reflect it's a TT error, not an internal one.
      (valerycodes)
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

Successfully merging this pull request may close these issues.

None yet

2 participants