If we're trying to view a remote event, we shouldn't pollute the logs#4847
If we're trying to view a remote event, we shouldn't pollute the logs#4847IgorA100 wants to merge 2 commits into
Conversation
… (view_video.php). For example, we were viewing an event, but the page in the browser wasn't closed, and in the meantime, the recorded event was deleted. In this case, there will be a persistent error in the logs.
- If a recorded event isn't found (eventNotFound == true), then print a message to the browser console and stop further script processing, as this is pointless, but it will help avoid unnecessary warnings and errors.
| // PHP variables to JS | ||
| // | ||
| var connKey = '<?php echo $connkey ?>'; | ||
| const eventNotFound = <?php echo ((!$Event->Id() || ($Event->Id() and !file_exists($Event->Path()))) ? "true" : "false") ?>; |
There was a problem hiding this comment.
The only problem here is that the event might exist on a different server. I would want to look for the file on the server that owns the storage area that the event is stored on. So..
If Event->Storage>ServerId() != currentServerId then ajax->checkforExistence().
There was a problem hiding this comment.
checkforExistence().
But how can we check a file on another server?
- We don't have access to the other server's file system, right?
- We haven't authenticated on the other server, and we can't pass the authorization token and name in the query string?
The multi-server configuration is still a mystery to me...
There was a problem hiding this comment.
@connortechnology
Moreover, I don't understand how a reference to a recorded event on another BB server could have ended up in the AA server database. I don't think that's possible...
There was a problem hiding this comment.
They share the same db
Oh, even so... I see, interesting, I didn't know that...
But I still have a question: what query can I use to check for a recorded event on another server? Isaac, have you done this before? If so, how?
There was a problem hiding this comment.
And again, I don't understand anything...
We have one storage for all servers, which means $Event->Path() will return the correct address.
For example:
there is Storage_A and Storage_B
there is Server_A and Server_B
Storage_A is connected to Server_A at /mnt/A/S_A
Storage_A is connected to Server_B at /mnt/B/S_A
Storage_B is connected to Server_B at /mnt/B/S_B
- When requesting an event stored on
Storage_AfromServer_A,$Event->Path()should return/mnt/A/S_A - When requesting an event stored on
Storage_AfromServer_B,$Event->Path()should return/mnt/B/S_A - When requesting an event stored on
Storage_BfromServer_A,$Event->Path()should return nothing, becauseStorage_Bisn't connected toServer_A. And we have no way to verify the presence of the recorded event.
Isn't that right?
There was a problem hiding this comment.
You have to use the API on the assigned server. Ideally all servers would have access to all storage, but sometimes that isn't true. Some people want to use a server just for the UI, multiple recording servers, etc. Sometimes located very differently on the network with slow unreliable network. So each recording server has it's own local storage which is reliable for recording, but may not be reliable (or performant) for playback. Which is why we can assign a ServerId to Storage to tell us where the best place to access it is. You have to ask the API on the server assigned to the Storage.
There was a problem hiding this comment.
Thanks for the clarification.
There was a problem hiding this comment.
Pull request overview
Reduces log noise when viewing an event whose recording is no longer available (e.g., deleted while a browser tab is still open), by downgrading a server-side error and short-circuiting the client-side init when the event isn't found.
Changes:
- Downgrade
ZM\ErrortoZM\Debugwhen the video file can't be opened inview_video.php. - Expose a new
eventNotFoundJS constant fromevent.js.phpbased on whether the Event has an Id and its path exists. - In
event.js, bail out early frominitPage()and log to console wheneventNotFoundis true.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| web/views/view_video.php | Changes failure log from Error to Debug when opening missing video. |
| web/skins/classic/views/js/event.js.php | Emits new eventNotFound JS constant based on event Id and path existence. |
| web/skins/classic/views/js/event.js | Short-circuits initPage() when the event is not found, logging to the console. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
For example, we were viewing an event, but the page in the browser wasn't closed, and in the meantime, the recorded event was deleted. In this case, there will be a persistent error in the logs.
The code on the Event page probably also needs to be changed.