Skip to content

Conversation

@paulbalandan
Copy link
Member

@paulbalandan paulbalandan commented Feb 12, 2022

Description
Closes #5677

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@paulbalandan paulbalandan force-pushed the refactor-exception-logging branch 2 times, most recently from 364ac02 to 8cee710 Compare February 12, 2022 17:07
@kenjis kenjis added the refactor Pull requests that refactor code label Feb 13, 2022
@paulbalandan paulbalandan force-pushed the refactor-exception-logging branch from 8cee710 to 491e505 Compare February 13, 2022 07:22
@paulbalandan paulbalandan force-pushed the refactor-exception-logging branch from 491e505 to d80309e Compare February 15, 2022 04:31
@kenjis
Copy link
Member

kenjis commented Feb 15, 2022

Why this PR is refactoring?
It seems it is enhancement.

@paulbalandan paulbalandan changed the title Refactor exception logging Improve exception logging Feb 15, 2022
@paulbalandan
Copy link
Member Author

Why this PR is refactoring? It seems it is enhancement.

Changed title.

@paulbalandan paulbalandan added enhancement PRs that improve existing functionalities and removed refactor Pull requests that refactor code labels Feb 15, 2022
@kenjis kenjis added the breaking change Pull requests that may break existing functionalities label Feb 15, 2022
@kenjis
Copy link
Member

kenjis commented Feb 15, 2022

Before:

CRITICAL - 2022-02-15 02:10:32 --> Cannot modify header information - headers already sent
#0 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler(2, 'Cannot modify h...', '/Users/kenji/wo...', 11)
#1 /Users/kenji/work/codeigniter/CodeIgniter4/app/Controllers/Home.php(11): setcookie('printer', '37')
#2 /Users/kenji/work/codeigniter/CodeIgniter4/system/CodeIgniter.php(834): App\Controllers\Home->index()
#3 /Users/kenji/work/codeigniter/CodeIgniter4/system/CodeIgniter.php(421): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Home))
#4 /Users/kenji/work/codeigniter/CodeIgniter4/system/CodeIgniter.php(329): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
#5 /Users/kenji/work/codeigniter/CodeIgniter4/public/index.php(37): CodeIgniter\CodeIgniter->run()
#6 /Users/kenji/work/codeigniter/CodeIgniter4/system/Commands/Server/rewrite.php(43): require_once('/Users/kenji/wo...')
#7 {main}

After:

CRITICAL - 2022-02-15 02:12:17 --> Cannot modify header information - headers already sent
in APPPATH/Controllers/Home.php on line 11.
 1 [internal function]: CodeIgniter\Debug\Exceptions->errorHandler(2, 'Cannot modify header information - headers already sent', 'APPPATH/Controllers/Home.php', 11)
 2 APPPATH/Controllers/Home.php(11): setcookie('printer', '37')
 3 SYSTEMPATH/CodeIgniter.php(834): App\Controllers\Home->index()
 4 SYSTEMPATH/CodeIgniter.php(421): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Home))
 5 SYSTEMPATH/CodeIgniter.php(329): CodeIgniter\CodeIgniter->handleRequest(null, Object(Config\Cache), false)
 6 FCPATH/index.php(37): CodeIgniter\CodeIgniter->run()
 7 SYSTEMPATH/Commands/Server/rewrite.php(43): require_once('FCPATH/index.php')


unset($frame['line']);
$idx = $index;
$idx = str_pad((string) ++$idx, 2, ' ', STR_PAD_LEFT);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you start with 1, not 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel that's more "natural" way of counting. No offense to zero-based array counting. 😂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$idx = str_pad((string) ++$idx, 2, '#', STR_PAD_LEFT);

This is like the original.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is like the original.

This would fail on idx >= 10. https://3v4l.org/DAqmo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://3v4l.org/dFGjg

echo '#'.str_pad(++$idx, 3, ' ', STR_PAD_RIGHT).'Some more text', PHP_EOL;

@kenjis
Copy link
Member

kenjis commented Feb 15, 2022

@paulbalandan The log format will be changed. If a user watches log files and parse it to check the error, it may break.
I think it is better to add to changelog.

@paulbalandan paulbalandan force-pushed the refactor-exception-logging branch from d80309e to 825534e Compare February 15, 2022 09:28
@paulbalandan paulbalandan merged commit 968c027 into codeigniter4:develop Feb 17, 2022
@paulbalandan paulbalandan deleted the refactor-exception-logging branch February 17, 2022 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Pull requests that may break existing functionalities enhancement PRs that improve existing functionalities

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CodeIgniter\Debug\Exceptions::exceptionHandler() - Add File and Line to message

3 participants