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

refactor: replace $e->getMessage() with $e in log_message() #6182

Merged
merged 2 commits into from
Jun 26, 2022

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jun 24, 2022

Description
Depending on exception classes, but generally (string) $e has more info than $e->getMessage().

If this is not BC, then I would change the base branch to develop.

$e->getMessage():

ERROR - 2022-06-23 19:09:15 --> Cookie object with name "unknown" and prefix "" was not found in the collection.

$e:

ERROR - 2022-06-23 19:09:31 --> CodeIgniter\Cookie\Exceptions\CookieException: Cookie object with name "unknown" and prefix "" was not found in the collection. in /Users/kenji/work/codeigniter/CodeIgniter4/system/Cookie/CookieStore.php:114
Stack trace:
#0 /Users/kenji/work/codeigniter/CodeIgniter4/system/Cookie/CookieStore.php(114): CodeIgniter\Cookie\Exceptions\CookieException::forUnknownCookieInstance(Array)
#1 /Users/kenji/work/codeigniter/CodeIgniter4/system/HTTP/ResponseTrait.php(635): CodeIgniter\Cookie\CookieStore->get('unknown', '')
#2 /Users/kenji/work/codeigniter/CodeIgniter4/app/Controllers/Home.php(14): CodeIgniter\HTTP\Response->getCookie('unknown')
#3 /Users/kenji/work/codeigniter/CodeIgniter4/system/CodeIgniter.php(862): App\Controllers\Home->index()
#4 /Users/kenji/work/codeigniter/CodeIgniter4/system/CodeIgniter.php(452): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Home))
#5 /Users/kenji/work/codeigniter/CodeIgniter4/system/CodeIgniter.php(346): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
#6 /Users/kenji/work/codeigniter/CodeIgniter4/public/index.php(55): CodeIgniter\CodeIgniter->run()
#7 /Users/kenji/work/codeigniter/CodeIgniter4/system/Commands/Server/rewrite.php(43): require_once('/Users/kenji/wo...')
#8 {main}

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

@kenjis kenjis added the 4.3 label Jun 24, 2022
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Nice touch. Modifying log entries does not affect our API, I would not consider this a breaking change at all - fine by me to switch branches.

Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

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

We have a user who is an advocate of declare(strict_types=1);. Can you cast to string the $e so that string is still passed to $message? I don't want to read another forum post or issue regarding this, please.

@MGatner
Copy link
Member

MGatner commented Jun 24, 2022

Exceptions are already Stringable: https://www.php.net/manual/en/exception.tostring.php

... so are allowed without casting.

@MGatner
Copy link
Member

MGatner commented Jun 24, 2022

Nope, you were right @paulbalandan:

If strict typing is enforced (with declare(strict_types=1) at the top of the file), an explicit string cast is required

I didn't realize that difference in behavior - unfortunate.

@kenjis kenjis changed the base branch from 4.3 to develop June 25, 2022 23:26
@kenjis kenjis changed the base branch from develop to 4.3 June 25, 2022 23:27
@kenjis kenjis changed the base branch from 4.3 to develop June 25, 2022 23:27
@kenjis kenjis removed the 4.3 label Jun 25, 2022
@kenjis kenjis requested a review from paulbalandan June 25, 2022 23:42
@kenjis kenjis added the refactor Pull requests that refactor code label Jun 25, 2022
@kenjis kenjis merged commit 74f12ec into codeigniter4:develop Jun 26, 2022
@kenjis kenjis deleted the fix-exception-logging branch June 26, 2022 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Pull requests that refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants