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

Feature Request: Add last error panel to bluescreen #133

Closed
wants to merge 1 commit into from

Conversation

@JanTvrdik
Copy link
Contributor

JanTvrdik commented Mar 2, 2016

In PHP it is quite common to use "shut-up + escalating error to exception" pattern (example). Unfortunately the original error message is lost unless error_get_last()['message'] is manually appended to exception message.

What about adding a simple panel to bluescreen to show the last error? The error may not always be related. We may attempt to do some magic to filter the relevant error but I'm not sure it's worth it.


Related: nette/utils#94

@hrach

This comment has been minimized.

Copy link
Contributor

hrach commented Jan 21, 2016

👍

2 similar comments
@fprochazka

This comment has been minimized.

Copy link
Contributor

fprochazka commented Jan 22, 2016

👍

@dg

This comment has been minimized.

Copy link
Member

dg commented Jan 22, 2016

👍

@fprochazka

This comment has been minimized.

Copy link
Contributor

fprochazka commented Jan 22, 2016

@JanTvrdik I would start simple and just implement it naively and if there are some cases where the info is irelevant, we might iterate on that after.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor Author

JanTvrdik commented Mar 1, 2016

Announcement to all talented programmers

This is your last chance to implement this awesome idea. If there will be no PR at 20.00 today, I'll do it myself and you will loose the unique opportunity to became famous for implementing this.

@dg

This comment has been minimized.

Copy link
Member

dg commented Mar 1, 2016

😀

@fprochazka

This comment has been minimized.

Copy link
Contributor

fprochazka commented Mar 1, 2016

super bussy ted

JanTvrdik added a commit to JanTvrdik/nette-tracy that referenced this pull request Mar 2, 2016
@JanTvrdik JanTvrdik force-pushed the JanTvrdik:pr/last_error_panel branch from 593921b to 70ef7f2 Mar 2, 2016
JanTvrdik added a commit to JanTvrdik/nette-tracy that referenced this pull request Mar 2, 2016
@JanTvrdik

This comment has been minimized.

Copy link
Contributor Author

JanTvrdik commented Mar 3, 2016

Any thoughts?

@fprochazka

This comment has been minimized.

Copy link
Contributor

fprochazka commented Mar 3, 2016

@JanTvrdik maybe it would be nice to hide the error, if the rendered exception is ErrorException created by nette handler and the error would be exactly the same?

@JanTvrdik

This comment has been minimized.

Copy link
Contributor Author

JanTvrdik commented Mar 3, 2016

@fprochazka I know but I don't see a simple and reliable way to write the condition. You may check ErrorException + file + line but it may have some false positives.

@JanTvrdik

This comment has been minimized.

Copy link
Contributor Author

JanTvrdik commented Mar 10, 2016

ping @dg, is this ok? should i fix tests?

<h2><a data-tracy-ref="^+" class="tracy-toggle tracy-collapsed">Last error</a></h2>
<div class="tracy-collapsed inner">

<h2><?= Helpers::errorTypeToString($error['type']) ?>: <?= htmlspecialchars($error['message'], ENT_IGNORE, 'UTF-8') ?></h2>

This comment has been minimized.

Copy link
@dg

dg Mar 10, 2016

Member

Inside H2 should be H3.

@@ -195,6 +195,23 @@ $code = $exception->getCode() ? ' #' . $exception->getCode() : '';
<?php endif ?>


<?php if ($error = error_get_last()): ?>

This comment has been minimized.

Copy link
@dg

dg Mar 18, 2016

Member

This should be saved sooner, because Tracy can rewrite it.


<h2><?= Helpers::errorTypeToString($error['type']) ?>: <?= htmlspecialchars($error['message'], ENT_IGNORE, 'UTF-8') ?></h2>
<?php if (isset($error['file']) && is_file($error['file'])): ?>
<p><?= Helpers::editorLink($error['file'], $error['line']) ?> <a data-tracy-ref="^+" class="tracy-toggle tracy-collapsed">source</a>&nbsp;</p>

This comment has been minimized.

Copy link
@dg

dg Mar 18, 2016

Member

This panel is collapsed, so the source code can be visible immediately (to eliminate the need to 2x click)

@dg dg force-pushed the nette:master branch from fba3e9c to e1e204c Mar 22, 2016
@dg dg force-pushed the nette:master branch 4 times, most recently from 5dd8d3e to 5ecd8e7 Apr 21, 2016
@dg dg force-pushed the nette:master branch 6 times, most recently from 7f3fd63 to bbc898e May 16, 2016
@JanTvrdik JanTvrdik closed this May 19, 2016
@JanTvrdik JanTvrdik force-pushed the JanTvrdik:pr/last_error_panel branch from d95bc4c to a7b64a4 May 19, 2016
WIP
@JanTvrdik JanTvrdik reopened this May 19, 2016
@JanTvrdik

This comment has been minimized.

Copy link
Contributor Author

JanTvrdik commented May 19, 2016

😫 I've put hours of energy into an attempt to fix the tests and it's still not done. This is ridiculous. 😫

@@ -95,6 +95,8 @@ private function renderTemplate($exception, $template)
$skipError = $sourceIsUrl && $exception instanceof \ErrorException && !empty($exception->skippable)
? $source . (strpos($source, '?') ? '&' : '?') . '_tracy_skip_error'
: NULL;
$lastError = (!$exception instanceof \ErrorException) ? error_get_last() : NULL;

This comment has been minimized.

Copy link
@dg

dg May 21, 2016

Member

$lastError = $exception instanceof \ErrorException ? NULL : error_get_last(); seems better

@dg dg force-pushed the nette:master branch from a566632 to 91fe6b8 May 27, 2016
@dg dg closed this in 3dbd632 May 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.