Page MenuHomeIn-Portal Phabricator

INP-1566 - Don't log errors from 3rd party libraries to System Log
ClosedPublic

Authored by alex on Jul 5 2016, 4:52 AM.

Details

Test Plan
NOTE: Perform tests with both Debugger enabled and disabled.

Preparations

  1. enable System Log in /system/config.php
  2. make sure In-Link module is installed
  3. make sure only In-Portal and Custom modules are enabled (in ConfigurationWebsiteModules section, when logged-in as root user)

Part 1

  • in IDE:
    1. add trigger_error('test admin', E_USER_WARNING); before $application->Run(); line in /admin/index.php file
    2. add trigger_error('test front-end', E_USER_WARNING); before $application->Run(); line in /index.php file
    3. create /vendor/behat/mink/error_test.php file with <?php trigger_error('test vendor', E_USER_WARNING); content
    4. create /../error_test.php (in parent folder of In-Portal root folder) file with <?php trigger_error('test outside', E_USER_WARNING); content
    5. create /modules/custom/error_test.php file with <?php trigger_error('test module', E_USER_WARNING); content
    6. create /modules/in-link/error_test.php file with <?php trigger_error('test disabled module', E_USER_WARNING); content
    7. create '/modules/unknown-module' folder
    8. create /modules/unknown-module/error_test.php file with <?php trigger_error('test unknown module', E_USER_WARNING); content
    9. create /error_test_vendor.php file as copy from /index.php file, that will have all code, that comes after $application->Init(); replaced with require FULL_PATH . '/vendor/behat/mink/error_test.php';
    10. create /error_test_outside.php file as copy from /index.php file, that will have all code, that comes after $application->Init(); replaced with require FULL_PATH . '/../error_test.php';
    11. create /error_test_module.php file as copy from /index.php file, that will have all code, that comes after $application->Init(); replaced with require MODULES_PATH . '/custom/error_test.php';
    12. create /error_test_disabled_module.php file as copy from /index.php file, that will have all code, that comes after $application->Init(); replaced with require MODULES_PATH . '/in-link/error_test.php';
    13. create /error_test_unknown_module.php file as copy from /index.php file, that will have all code, that comes after $application->Init(); replaced with require MODULES_PATH . '/unknown-module/error_test.php';
  • in Web Browser:
    1. open /admin/index.php url
    2. confirm, that test admin error was logged into System Log (always) and Debugger (if enabled)
    3. open /index.php url
    4. confirm, that test front-end error was logged into System Log (always) and Debugger (if enabled)
    5. open /error_test_vendor.php url
    6. confirm, that nothing was logged in System Log and Debugger
    7. open /error_test_outside.php url
    8. confirm, that nothing was logged in System Log and Debugger
    9. open /error_test_unknown_module.php url
    10. confirm, that nothing was logged in System Log and Debugger
    11. open /error_test_disabled_module.php url
    12. confirm, that nothing was logged in System Log and Debugger
    13. open /error_test_module.php url
    14. confirm, that test module error was logged into System Log (always) and Debugger (if enabled)

Part 2

  • in IDE:
    1. in all created PHP files replace trigger_error(..., E_USER_WARNING) function call with throw new Exception(...);
  • in Web Browser:
    1. open /admin/index.php url
    2. confirm, that test admin exception was logged into System Log (always) and Debugger (if enabled)
    3. open /index.php url
    4. confirm, that test front-end exception was logged into System Log (always) and Debugger (if enabled)
    5. open /error_test_vendor.php url
    6. confirm, that test vendor exception was logged into System Log (always) and Debugger (if enabled)
    7. open /error_test_outside.php url
    8. confirm, that test outside exception was logged into System Log (always) and Debugger (if enabled)
    9. open /error_test_unknown_module.php url
    10. confirm, that test unknown module exception was logged into System Log (always) and Debugger (if enabled)
    11. open /error_test_disabled_module.php url
    12. confirm, that test disabled module exception was logged into System Log (always) and Debugger (if enabled)
    13. open /error_test_module.php url
    14. confirm, that test module exception was logged into System Log (always) and Debugger (if enabled)

Part 3

  • in IDE:
    1. in all created PHP files replace throw new Exception(...) function call with trigger_error(..., E_USER_ERROR);
  • in Web Browser:
    1. open /admin/index.php url
    2. confirm, that test admin fatal error was logged into System Log (always) and Debugger (if enabled)
    3. open /index.php url
    4. confirm, that test front-end fatal error was logged into System Log (always) and Debugger (if enabled)
    5. open /error_test_vendor.php url
    6. confirm, that test vendor fatal error was logged into System Log (always) and Debugger (if enabled)
    7. open /error_test_outside.php url
    8. confirm, that test outside fatal error was logged into System Log (always) and Debugger (if enabled)
    9. open /error_test_unknown_module.php url
    10. confirm, that test unknown module fatal error was logged into System Log (always) and Debugger (if enabled)
    11. open /error_test_disabled_module.php url
    12. confirm, that test disabled module fatal error was logged into System Log (always) and Debugger (if enabled)
    13. open /error_test_module.php url
    14. confirm, that test module fatal error was logged into System Log (always) and Debugger (if enabled)

Diff Detail

Repository
rINP In-Portal
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

alex updated this revision to Diff 545.Jul 5 2016, 4:52 AM
alex retitled this revision from to INP-1566 - Don't log errors from 3rd party libraries to System Log.
alex updated this object.
alex edited the test plan for this revision. (Show Details)
alex added 1 JIRA issue(s): INP-1566.
alex edited the test plan for this revision. (Show Details)Jul 5 2016, 4:53 AM
alex edited edge metadata.
erik accepted this revision.Jul 6 2016, 4:30 AM
erik edited edge metadata.
This revision is now accepted and ready to land.Jul 6 2016, 4:30 AM
alex edited the test plan for this revision. (Show Details)Jul 6 2016, 4:58 AM
alex edited edge metadata.
alex planned changes to this revision.Jul 15 2016, 9:10 AM
alex added inline comments.
core/kernel/utility/logger.php
724 ↗(On Diff #545)

We should additionally check that LogSourceFilename key exists in the $this->_logRecord array.

alex updated this revision to Diff 575.Jul 15 2016, 9:12 AM
alex edited the test plan for this revision. (Show Details)

Account for cases, when error could missing source filename.

This revision is now accepted and ready to land.Jul 15 2016, 9:12 AM
alex requested a review of this revision.Jul 15 2016, 9:13 AM
alex marked an inline comment as done.
alex edited edge metadata.

Please re-test.

erik added a comment.Jul 25 2016, 4:57 AM

Testing partially done. Some tests failed.

On disabled debug mode failed Part1 tests:

open /error_test_vendor.php url
confirm, that nothing was logged in System Log and Debugger
open /error_test_outside.php url
confirm, that nothing was logged in System Log and Debugger
open /error_test_unknown_module.php url
confirm, that nothing was logged in System Log and Debugger
open /error_test_disabled_module.php url
confirm, that nothing was logged in System Log and Debugger

in all cases above was logged in System log.

On enabled debug mode failed Part3 tests:

open /error_test_vendor.php url
confirm, that test vendor fatal error was logged into System Log (always) and Debugger (if enabled)
open /error_test_outside.php url
confirm, that test outside fatal error was logged into System Log (always) and Debugger (if enabled)
open /error_test_unknown_module.php url
confirm, that test unknown module fatal error was logged into System Log (always) and Debugger (if enabled)
open /error_test_disabled_module.php url
confirm, that test disabled module fatal error was logged into System Log (always) and Debugger (if enabled)

in all cases above was not logged in System log.

erik requested changes to this revision.Aug 2 2016, 6:02 AM
erik edited edge metadata.

All tests done. No more errors found, excepting reported on Mon, Jul 25, 04:57.

This revision now requires changes to proceed.Aug 2 2016, 6:02 AM
alex updated this revision to Diff 685.Jan 20 2017, 6:31 AM
alex edited edge metadata.

Fixing cases, when error that should be logged wasn't logged and vice versa.

erik accepted this revision.Jan 30 2017, 4:45 AM
erik edited edge metadata.
This revision is now accepted and ready to land.Jan 30 2017, 4:45 AM
This revision was automatically updated to reflect the committed changes.