It might be better to instead change core/SupportedBrowser.php to not throw an exception but instead to set a 403 HTTP header, output the message, and exit within that file.

@mwithheld to change the response code for this exception we could adjust the NotSupportedBrowserException to extend HttpCodeException

@sgiehl While true and a good idea, I'm proposing that this is not an Exception and so should not be handled like one -- it's normal program flow that should just [set a 403 HTTP header, output the message, and exit] (there's no code for this in core/SupportedBrowser.php). It's more a design decision if you want to use the exception handling anyway - and that'd be fine IMO -- I just don't feel it's my choice to make.

Guess using an exception makes it easy to automatically have the error screen rendered. Otherwise this would need to be triggered manually. But I agree that it's useless to log that exception. Personally I would keep the exception and maybe extend our exception with an additional flag that makes it possible to make the exception as "do not log". But I'm also fine with any other solution. Maybe @tsteur or @diosmosis have a stronger opinion on that?

While it's only one or two exceptions and it be good to do it like in the PR for simplicity 👍 . And when it's needed later down the road then we can still go for the more complex solution of having a flag or so.

Random exits in the code makes tests unpredictable (eg, I just spent 2-3 days debugging a travis only test failure that was happening because our redirect logic just exits, and one test was triggering it but only on travis) and makes it much harder to debug user issues where the exit is triggered especially when we don't have access to the users install and/or they are non-technical while throwing an exception means we can get a stack trace. It would make sense to me to use HttpCodeException and in ExceptionHandler ignore those or log them at a debug level.

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@mwithheld might it be possible that you pushed the latest changes to the wrong branch? Seems they are not included in this PR

Those commits are in patch-4 but not coming through in the PR for some reason. I've instead created PR #17915

Please see #17915 awaiting review.

Seems both PRs meanwhile have the same changes. Will close this on again in favor of #17915