Apply max execution query time to Live.getCounters API and queryAdjacentVisitorId method #18852

Description:

Noticed the max execution query configuration is not applied to live counters which it is now. When the query takes too long, then it will show a dash and a tooltip explaining that it took too long to get the data. The message may vary depending if a segment is selected or not

For queryAdjacentVisitorId if there's a timeout we simply assume that no next or previous visitor was found.

Checked https://matomo.org/faq/how-to/how-can-i-automatically-stop-long-running-database-queries/ which doesn't need to be updated by the looks.

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
• [ ] Wording review done
• [ ] 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

@sgiehl can you confirm you tested it and not just looked at the code? I tested it too but be good to have confirmation

@tsteur Yes, local testing is always part of a review. But I actually only had a look at the "Visits in Real-time" widget.

What could be improved would be the error handling of the "Real Time Visitor Count" widget. As it currently shows this when it exceeds the execution time:

And if the first request works but a subsequent request fails, it shows a global notification with the error message.

👍 That should be fixed now @sgiehl

Found another widget that is affected by the change. We have a widget Visitor profile, which currently has the same behavior as the Real time visitor count before.
I guess the problem is that the API method to get the profile throws a execution time error while fetching the latest visitorid, which isn't handled in the controller here:
https://github.com/matomo-org/matomo/blob/285bbfd022e2e23728daa453b6876d51061af018/plugins/Live/Controller.php#L169

@sgiehl I couldn't reproduce this one, or at least not due to this change. It might be an existing issue? We could create a new issue for it? The visitor query is handled in the private Live/Model::handleAdjacentVisitorIds() query so I don't think it can cause an issue and couldn't reproduce it for that query that was changed.

@tsteur but VisitorProfile::handleAdjacentVisitorIds() calls Live/Model::queryAdjacentVisitorId, which has been changed:
https://github.com/matomo-org/matomo/blob/115527353a9e75e01aa4d263408956ae45403bea/plugins/Live/VisitorProfile.php#L94-L97

@sgiehl this should be handled in https://github.com/matomo-org/matomo/pull/18852/files#diff-e3bdcefa5a3431d680f8e0d579f6d4f9be13cc944663b725983b7a703465d6aeR94 ? Or is that not working for you?

Had another look at that. I'm still seeing the error. Debugged it though, and the problem seems to be this:

https://github.com/matomo-org/matomo/blob/8770ed05a81c9f47c28231bdbc63693cfe2a09c0/plugins/Live/API.php#L220

@sgiehl that would be an existing error and not related to this PR?

@tsteur Than it might be a regression from the first PR introducing the query time limit for live reports.
Feel free to fix it here. Or merge this one and create a follow up issue.

created https://github.com/matomo-org/matomo/issues/18900