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

Improve handling of NotSupportedBrowserException #17915

Merged
merged 2 commits into from Sep 14, 2021
Merged

Improve handling of NotSupportedBrowserException #17915

merged 2 commits into from Sep 14, 2021

Conversation

mwithheld
Copy link
Contributor

@mwithheld mwithheld commented Aug 24, 2021

Description:

fixes #17738 Don't log an error when not supported browser is used

To reproduce the error (before the changes in this ticket):

  • Monitor the server (e.g. Apache) error log, e.g. tail -f error_log
  • Do a GET on matomo with an unsupported browser version e.g. Firefox < version 32, e.g.
    wget -U "Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0" --no-check-certificate https://127.0.0.1/matomo/

To test the fix:

  • Repeat the steps to reproduce above - you should not see the message in the error log.
  • In config.ini.php, set loglevel=DEBUG (the default is WARN):
[log]
log_level = DEBUG
  • Repeat the steps to reproduce above - you should now see the message in the error log.

Review

  • Functional review done
  • Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2021

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Sep 8, 2021
@mwithheld
Copy link
Contributor Author

Dealt with in PR #17812

@mwithheld mwithheld closed this Sep 8, 2021
@mwithheld
Copy link
Contributor Author

Hm no it's not dealt with after all - re-opening.

@mwithheld mwithheld reopened this Sep 8, 2021
@sgiehl sgiehl added Needs Review PRs that need a code review and removed Stale The label used by the Close Stale Issues action labels Sep 8, 2021
@sgiehl sgiehl added this to the 4.5.0 milestone Sep 8, 2021
core/ExceptionHandler.php Outdated Show resolved Hide resolved
Co-authored-by: Stefan Giehl <stefan@matomo.org>
@sgiehl sgiehl merged commit ea76729 into matomo-org:4.x-dev Sep 14, 2021
@mwithheld mwithheld deleted the 17812 branch September 15, 2021 19:18
@justinvelluppillai justinvelluppillai changed the title #17812 Improve handling of NotSupportedBrowserException Improve handling of NotSupportedBrowserException Oct 6, 2021
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't log an error when not supported browser is used
3 participants