Add line and file to the exception message when debug is ON #11675

ggppdk · 2016-08-19T20:36:27Z

Pull Request for Issue #10732

Summary of Changes

This PR adds the last file and line,
of the file at which the error was thrown
when debug is ON

A backtrace without it was added by this PR (when debug is ON)
#10964

Testing Instructions

Cause an exception to be thrown at a file

The file and line, should show up in the error message (when debug is ON)

Documentation Changes Required

ggppdk · 2016-08-19T21:40:34Z

@mbabker

When debug parameter is ON, a lot get displayed, besides the recently added backtrace,
the DEBUG console shows a lot
and not no website will really run with it DEBUG on, it makes sense to add this

andrepereiradasilva · 2016-08-19T22:55:10Z

to me this sound a little confusing ...

IMHO this would be a better to have it on the backtrace ... maybe the numbering also inverted?

I actually prefer the debug plugin Log call back

ggppdk · 2016-08-19T23:00:46Z

Yes i had similar thoughts ...

but before anything, we could agree that it is ok if this get displayed when debug parameter is ON:

When debug parameter is ON, a lot get displayed, besides the recently added backtrace,
the DEBUG console shows a lot
and not no website will really run with it DEBUG on, it makes sense to add this

andrepereiradasilva · 2016-08-19T23:05:35Z

but before anything, we could agree that it is ok if this get displayed when debug parameter is ON:

i see no problem with that.

When debug parameter is ON, a lot get displayed, besides the recently added backtrace,
the DEBUG console shows a lot
and not no website will really run with it DEBUG on, it makes sense to add this

IMO no production website should ever be with debug set to ON in global config or/and the system debug plugin enabled.

mbabker · 2016-08-19T23:06:46Z

The backtrace that's rendered here comes from the Throwable object, which
doesn't include the line it was created at. The debug plugin uses
debug_backtrace() for its traces, different API with different behavior.

On Friday, August 19, 2016, andrepereiradasilva notifications@github.com
wrote:

to me this sound a little confusing ...

[image: image]
https://cloud.githubusercontent.com/assets/9630530/17826330/8eb14678-6667-11e6-90f2-0882c990750d.png

IMHO this would be a better to have it on the backtrace ... maybe the
numbering also inverted?

I actually prefer the debug plugin Log call back

[image: image]
https://cloud.githubusercontent.com/assets/9630530/17826396/42ee7c3c-6668-11e6-8447-138bd44cc4f4.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11675 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXJnGVIj44xXBxl9g8qm3vHjD7fIks5qhjRPgaJpZM4Jo1QN
.

ggppdk · 2016-08-19T23:16:06Z

@mbabker

The backtrace that's rendered here comes from the Throwable object, which doesn't include the line it was created at.

ok, so this, you say this should not be modified to include it :?
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/document/error.php#L197

What about (1) adding it the way this PR suggests:

IMO no production website should ever be with debug set to ON in global config or/and the system debug plugin enabled.

and (2) what about changing method signature (so that we can render trace in reverse):

public function renderBacktrace()

to

public function renderBacktrace($reverse = false)

mbabker · 2016-08-19T23:21:37Z

I wouldn't add it to the render method right now. Personally I'd go for
refactoring to inject a Throwable into it versus relying on the internal
pointer (it's why the stacked Exception PR added so many lines to the
template) and finish the PR that moves rendering to a layout and let the
order be a design decision. But the former's a 4.0 thing, or creating a
more generic and reusable helper function for rendering traces in general
with the object injected correctly.

On Friday, August 19, 2016, Georgios Papadakis notifications@github.com
wrote:

@mbabker https://github.com/mbabker

The backtrace that's rendered here comes from the Throwable object, which
doesn't include the line it was created at.

ok, so this, you say this should not be modified to include it :?
https://github.com/joomla/joomla-cms/blob/staging/
libraries/joomla/document/error.php#L197

What about (1) adding it the way this PR suggest:

IMO no production website should ever be with debug set to ON in global
config or/and the system debug plugin enabled.

and (2) what about changing method signature of (so that we can render
trace in reverse):

public function renderBacktrace()

to

public function renderBacktrace($reverse = false)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11675 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfobQliMq6Y4uqFH3DKXkt7Xw_h2TWks5qhjk4gaJpZM4Jo1QN
.

brianteeman · 2016-08-26T10:02:08Z

@ggppdk Where are we at with this - is it on hold?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11675.}

ggppdk · 2016-08-26T20:12:28Z

Well noone objected for adding this file and line of the error when DEBUG is ON,

there was a concern for doing it in more proper place,
but this is not at renderBacktrace() as @mbabker said,

so i think we are good to test it
(unless someone has a better suggestion)

Hackwar · 2017-05-24T19:04:23Z

So, I ran into this today and even opened an issue as you can see. Can you update the branch to fix the merge conflict? Then we should test this. :-)

Hackwar · 2017-05-24T19:05:30Z

To clarify: I'd rather implement this the way it is proposed here, than to have the broken display that we have right now. If there is a better solution, we can still implement that in 4.0.

Hackwar · 2017-05-30T07:31:48Z

The changes to the error.php files do work, but unfortunately another change crept into this PR. Can you fix that and also again merge in the staging branch?

ggppdk · 2017-05-30T11:31:58Z

PR updated

Quy · 2017-05-30T12:06:18Z

administrator/templates/system/error.php

-        </p>
+					<?php echo htmlspecialchars($this->error->getMessage(), ENT_QUOTES, 'UTF-8'); ?>
+					<?php if ($this->debug) : ?>
+						<br/><?php echo htmlspecialchars($this->error->getFile(), ENT_QUOTES, 'UTF-8');?>:<?php echo $this->error->getLine(); ?>


Change <br/> to <br>.

You may want to change to this for better readability:
<?php echo htmlspecialchars($this->error->getFile(), ENT_QUOTES, 'UTF-8') . ':' . $this->error->getLine(); ?>

Hackwar · 2017-06-17T13:49:02Z

Can you add this code to the other error.php files in the frontend, too?

ggppdk · 2017-06-18T12:16:59Z

PR updated to add these changes to the error.php of the frontend templates

DrDreave · 2017-08-21T09:00:52Z

I have tested this item ✅ successfully on 4d54887

Operating System

Joomla! 3.8.0-beta3-dev
PHP 5.6.2
MySQLi 5.5.38
Apache/2.2.29 (Unix)

Steps

Enable Debug mode
Cause component error by calling a non-existent component (e.g. index.php?option=com_xxx)

Test before patch

Error message and stack trace shown (see image attached below)

Test after patch

Error message naming faulty statement and stack trace shown (see image attached below)
Disabling the Debug Mode hides the message

Tested @icampus

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11675.}

ghost · 2017-08-21T09:48:27Z

@lavipr please mark your Test at Issue Tracker.

If this is not possible, i can alter Test.

lavipr · 2017-08-21T09:51:04Z

I have tested this item ✅ successfully on 4d54887

I have tested this patch succesfully. Installed is latest joomla version from github with sample data.

Steps:

enable debug mode
caused a type error of "this" in backend view contacts
Test before Patch : The file and line, don't show up in the error message
Test after Patch: The file and line, are displayed in the error message

Tested @icampus

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11675.}

ghost · 2017-08-21T09:59:31Z

RTC after two successful tests.

Crometor · 2017-08-21T10:35:36Z

I have tested this item ✅ successfully on 4d54887

I followed the Steps, explained by DrDreave and lavipr I got the same result.

Tested @icampus

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11675.}

ghost · 2017-08-21T10:39:38Z

@Crometor thanks for Test this Pull Request. As it has 2 successfully Tests, it wasn't necessary for this PR for 3rd Test :-)

joomla-cms-bot added the PR-staging label Aug 19, 2016

ggppdk mentioned this pull request Aug 19, 2016

Component options dialog - "permissions" tab mandatory? #11668

Closed

infograf768 force-pushed the staging branch from cfb8cfd to 59a9426 Compare May 25, 2017 09:51

Quy reviewed May 30, 2017

View reviewed changes

ggppdk and others added 4 commits June 17, 2017 23:22

Add Line and File printing to the error message

b82a86f

Add Line and File printing to the error message

45959c7

Add Line and File printing to the error message

428df82

Updated frontend error files, and code for previous errors too

4d54887

ggppdk force-pushed the patch-25 branch from 7f790aa to 4d54887 Compare June 17, 2017 21:06

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 21, 2017

mbabker added this to the Joomla 3.8.0 milestone Aug 21, 2017

mbabker merged commit 848cfe1 into joomla:staging Aug 21, 2017

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add line and file to the exception message when debug is ON #11675

Add line and file to the exception message when debug is ON #11675

ggppdk commented Aug 19, 2016 •

edited by joomla-cms-bot

ggppdk commented Aug 19, 2016

andrepereiradasilva commented Aug 19, 2016

ggppdk commented Aug 19, 2016

andrepereiradasilva commented Aug 19, 2016

mbabker commented Aug 19, 2016

ggppdk commented Aug 19, 2016 •

edited

mbabker commented Aug 19, 2016

brianteeman commented Aug 26, 2016

ggppdk commented Aug 26, 2016

Hackwar commented May 24, 2017

Hackwar commented May 24, 2017

Hackwar commented May 30, 2017

ggppdk commented May 30, 2017

Quy May 30, 2017

Hackwar commented Jun 17, 2017

ggppdk commented Jun 18, 2017

DrDreave commented Aug 21, 2017 •

edited

ghost commented Aug 21, 2017 •

edited by ghost

lavipr commented Aug 21, 2017

ghost commented Aug 21, 2017

Crometor commented Aug 21, 2017

ghost commented Aug 21, 2017

Add line and file to the exception message when debug is ON #11675

Add line and file to the exception message when debug is ON #11675

Conversation

ggppdk commented Aug 19, 2016 • edited by joomla-cms-bot

Summary of Changes

Testing Instructions

Documentation Changes Required

ggppdk commented Aug 19, 2016

andrepereiradasilva commented Aug 19, 2016

ggppdk commented Aug 19, 2016

andrepereiradasilva commented Aug 19, 2016

mbabker commented Aug 19, 2016

ggppdk commented Aug 19, 2016 • edited

mbabker commented Aug 19, 2016

brianteeman commented Aug 26, 2016

ggppdk commented Aug 26, 2016

Hackwar commented May 24, 2017

Hackwar commented May 24, 2017

Hackwar commented May 30, 2017

ggppdk commented May 30, 2017

Quy May 30, 2017

Choose a reason for hiding this comment

Hackwar commented Jun 17, 2017

ggppdk commented Jun 18, 2017

DrDreave commented Aug 21, 2017 • edited

Operating System

Steps

Test before patch

Test after patch

ghost commented Aug 21, 2017 • edited by ghost

lavipr commented Aug 21, 2017

ghost commented Aug 21, 2017

Crometor commented Aug 21, 2017

ghost commented Aug 21, 2017

ggppdk commented Aug 19, 2016 •

edited by joomla-cms-bot

ggppdk commented Aug 19, 2016 •

edited

DrDreave commented Aug 21, 2017 •

edited

ghost commented Aug 21, 2017 •

edited by ghost