mirror of https://github.com/OpenVidu/openvidu.git
openvidu-server: fix missing recordingStatusChanged failed event
parent
ffd232b428
commit
a002e766ca
|
|
@ -102,7 +102,7 @@ public class ComposedQuickStartRecordingService extends ComposedRecordingService
|
||||||
this.sessionsContainers.put(session.getSessionId(), containerId);
|
this.sessionsContainers.put(session.getSessionId(), containerId);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
this.waitForVideoFileNotEmpty(recording);
|
this.waitForVideoFileNotEmpty(recording, session);
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
this.cleanRecordingMaps(recording);
|
this.cleanRecordingMaps(recording);
|
||||||
throw this.failStartRecording(session, recording,
|
throw this.failStartRecording(session, recording,
|
||||||
|
|
@ -133,13 +133,18 @@ public class ComposedQuickStartRecordingService extends ComposedRecordingService
|
||||||
recording.getId());
|
recording.getId());
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
if (containerId != null) {
|
||||||
dockerManager.runCommandInContainerSync(recording.getRecordingProperties().mediaNode(), containerId,
|
try {
|
||||||
"./composed_quick_start.sh --stop-recording", 10);
|
dockerManager.runCommandInContainerSync(recording.getRecordingProperties().mediaNode(), containerId,
|
||||||
} catch (IOException e1) {
|
"./composed_quick_start.sh --stop-recording", 10);
|
||||||
log.error("Error stopping COMPOSED_QUICK_START recording {}: {}", recording.getId(), e1.getMessage());
|
} catch (IOException e1) {
|
||||||
failRecordingCompletion(recording, containerId, true,
|
log.error("Error stopping COMPOSED_QUICK_START recording {}: {}", recording.getId(), e1.getMessage());
|
||||||
new OpenViduException(Code.RECORDING_COMPLETION_ERROR_CODE, e1.getMessage()));
|
failRecordingCompletion(recording, containerId, true,
|
||||||
|
new OpenViduException(Code.RECORDING_COMPLETION_ERROR_CODE, e1.getMessage()));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
log.warn("No container ID found for session {} when stopping recording {}. Container may have already been cleaned up or never created.",
|
||||||
|
recording.getSessionId(), recording.getId());
|
||||||
}
|
}
|
||||||
|
|
||||||
if (this.openviduConfig.isRecordingComposedExternal()) {
|
if (this.openviduConfig.isRecordingComposedExternal()) {
|
||||||
|
|
|
||||||
|
|
@ -175,7 +175,7 @@ public class ComposedRecordingService extends RecordingService {
|
||||||
this.sessionsContainers.put(session.getSessionId(), containerId);
|
this.sessionsContainers.put(session.getSessionId(), containerId);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
this.waitForVideoFileNotEmpty(recording);
|
this.waitForVideoFileNotEmpty(recording, session);
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
this.cleanRecordingMaps(recording);
|
this.cleanRecordingMaps(recording);
|
||||||
throw this.failStartRecording(session, recording,
|
throw this.failStartRecording(session, recording,
|
||||||
|
|
@ -397,9 +397,22 @@ public class ComposedRecordingService extends RecordingService {
|
||||||
}
|
}
|
||||||
|
|
||||||
protected void updateRecordingAttributes(Recording recording) {
|
protected void updateRecordingAttributes(Recording recording) {
|
||||||
|
String infoFilePath = this.openviduConfig.getOpenViduRecordingPath()
|
||||||
|
+ recording.getId() + "/" + recording.getId() + RecordingService.COMPOSED_INFO_FILE_EXTENSION;
|
||||||
|
|
||||||
|
// Check if info file exists before trying to process it
|
||||||
|
java.nio.file.Path path = java.nio.file.Paths.get(infoFilePath);
|
||||||
|
if (!java.nio.file.Files.exists(path)) {
|
||||||
|
log.warn("Recording info file does not exist at {}. Recording {} may have failed to start properly.",
|
||||||
|
infoFilePath, recording.getId());
|
||||||
|
recording.setStatus(io.openvidu.java.client.Recording.Status.failed);
|
||||||
|
// Don't return early - we need to continue so that sealRecordingMetadataFileAsReady()
|
||||||
|
// gets called in downloadComposedRecording(), which triggers the webhook
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
RecordingInfoUtils infoUtils = new RecordingInfoUtils(this.openviduConfig.getOpenViduRecordingPath()
|
RecordingInfoUtils infoUtils = new RecordingInfoUtils(infoFilePath);
|
||||||
+ recording.getId() + "/" + recording.getId() + RecordingService.COMPOSED_INFO_FILE_EXTENSION);
|
|
||||||
|
|
||||||
if (!infoUtils.hasVideo()) {
|
if (!infoUtils.hasVideo()) {
|
||||||
log.error("COMPOSED recording {} with hasVideo=true has not video track", recording.getId());
|
log.error("COMPOSED recording {} with hasVideo=true has not video track", recording.getId());
|
||||||
|
|
@ -421,11 +434,14 @@ public class ComposedRecordingService extends RecordingService {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
protected void waitForVideoFileNotEmpty(Recording recording) throws Exception {
|
protected void waitForVideoFileNotEmpty(Recording recording, Session session) throws Exception {
|
||||||
final String VIDEO_FILE = this.openviduConfig
|
final String VIDEO_FILE = this.openviduConfig
|
||||||
.getOpenViduRecordingPath(recording.getRecordingProperties().mediaNode()) + recording.getId() + "/"
|
.getOpenViduRecordingPath(recording.getRecordingProperties().mediaNode()) + recording.getId() + "/"
|
||||||
+ recording.getName() + RecordingService.COMPOSED_RECORDING_EXTENSION;
|
+ recording.getName() + RecordingService.COMPOSED_RECORDING_EXTENSION;
|
||||||
this.fileManager.waitForFileToExistAndNotEmpty(recording.getRecordingProperties().mediaNode(), VIDEO_FILE);
|
|
||||||
|
// Check if session was closed while we're waiting for the video file
|
||||||
|
fileManager.waitForFileToExistAndNotEmpty(recording.getRecordingProperties().mediaNode(), VIDEO_FILE,
|
||||||
|
() -> !session.isClosed());
|
||||||
log.info("File {} exists and is not empty", VIDEO_FILE);
|
log.info("File {} exists and is not empty", VIDEO_FILE);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -201,6 +201,11 @@ public abstract class RecordingService {
|
||||||
|
|
||||||
this.recordingManager.startingRecordings.remove(recording.getId());
|
this.recordingManager.startingRecordings.remove(recording.getId());
|
||||||
this.recordingManager.sessionsRecordingsStarting.remove(session.getSessionId());
|
this.recordingManager.sessionsRecordingsStarting.remove(session.getSessionId());
|
||||||
|
|
||||||
|
// Send "failed" webhook event
|
||||||
|
this.cdr.recordRecordingStatusChanged(recording, null, System.currentTimeMillis(),
|
||||||
|
io.openvidu.java.client.Recording.Status.failed);
|
||||||
|
|
||||||
this.stopRecording(session, recording, null);
|
this.stopRecording(session, recording, null);
|
||||||
return new OpenViduException(Code.RECORDING_START_ERROR_CODE, errorMessage);
|
return new OpenViduException(Code.RECORDING_START_ERROR_CODE, errorMessage);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -23,6 +23,7 @@ import java.io.FileOutputStream;
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.io.OutputStreamWriter;
|
import java.io.OutputStreamWriter;
|
||||||
import java.util.Properties;
|
import java.util.Properties;
|
||||||
|
import java.util.function.BooleanSupplier;
|
||||||
|
|
||||||
import org.apache.commons.io.FileUtils;
|
import org.apache.commons.io.FileUtils;
|
||||||
import org.slf4j.Logger;
|
import org.slf4j.Logger;
|
||||||
|
|
@ -123,6 +124,9 @@ public abstract class CustomFileManager {
|
||||||
|
|
||||||
public abstract void waitForFileToExistAndNotEmpty(String mediaNodeId, String absolutePathToFile) throws Exception;
|
public abstract void waitForFileToExistAndNotEmpty(String mediaNodeId, String absolutePathToFile) throws Exception;
|
||||||
|
|
||||||
|
public abstract void waitForFileToExistAndNotEmpty(String mediaNodeId, String absolutePathToFile,
|
||||||
|
BooleanSupplier shouldContinueWaiting) throws Exception;
|
||||||
|
|
||||||
public abstract int maxSecondsWaitForFile();
|
public abstract int maxSecondsWaitForFile();
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,11 +1,18 @@
|
||||||
package io.openvidu.server.utils;
|
package io.openvidu.server.utils;
|
||||||
|
|
||||||
import java.io.File;
|
import java.io.File;
|
||||||
|
import java.util.function.BooleanSupplier;
|
||||||
|
|
||||||
public class LocalCustomFileManager extends CustomFileManager {
|
public class LocalCustomFileManager extends CustomFileManager {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void waitForFileToExistAndNotEmpty(String mediaNodeId, String absolutePathToFile) throws Exception {
|
public void waitForFileToExistAndNotEmpty(String mediaNodeId, String absolutePathToFile) throws Exception {
|
||||||
|
waitForFileToExistAndNotEmpty(mediaNodeId, absolutePathToFile, () -> true);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void waitForFileToExistAndNotEmpty(String mediaNodeId, String absolutePathToFile,
|
||||||
|
BooleanSupplier shouldContinueWaiting) throws Exception {
|
||||||
|
|
||||||
// Check 10 times per seconds
|
// Check 10 times per seconds
|
||||||
int MAX_SECONDS_WAIT = this.maxSecondsWaitForFile();
|
int MAX_SECONDS_WAIT = this.maxSecondsWaitForFile();
|
||||||
|
|
@ -14,7 +21,7 @@ public class LocalCustomFileManager extends CustomFileManager {
|
||||||
int i = 0;
|
int i = 0;
|
||||||
|
|
||||||
boolean arePresent = fileExistsAndHasBytes(absolutePathToFile);
|
boolean arePresent = fileExistsAndHasBytes(absolutePathToFile);
|
||||||
while (!arePresent && i < LIMIT) {
|
while (!arePresent && i < LIMIT && shouldContinueWaiting.getAsBoolean()) {
|
||||||
try {
|
try {
|
||||||
Thread.sleep(MILLISECONDS_INTERVAL_WAIT);
|
Thread.sleep(MILLISECONDS_INTERVAL_WAIT);
|
||||||
arePresent = fileExistsAndHasBytes(absolutePathToFile);
|
arePresent = fileExistsAndHasBytes(absolutePathToFile);
|
||||||
|
|
@ -23,6 +30,9 @@ public class LocalCustomFileManager extends CustomFileManager {
|
||||||
throw new Exception("Interrupted exception while waiting for file " + absolutePathToFile + " to exist");
|
throw new Exception("Interrupted exception while waiting for file " + absolutePathToFile + " to exist");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (!shouldContinueWaiting.getAsBoolean()) {
|
||||||
|
throw new Exception("Recording was stopped while waiting for file " + absolutePathToFile);
|
||||||
|
}
|
||||||
if (!arePresent) {
|
if (!arePresent) {
|
||||||
throw new Exception("File " + absolutePathToFile + " does not exist and hasn't been created in "
|
throw new Exception("File " + absolutePathToFile + " does not exist and hasn't been created in "
|
||||||
+ MAX_SECONDS_WAIT + " seconds");
|
+ MAX_SECONDS_WAIT + " seconds");
|
||||||
|
|
|
||||||
|
|
@ -1523,44 +1523,22 @@ public class OpenViduTestAppE2eTest extends AbstractOpenViduTestappE2eTest {
|
||||||
user.getEventManager().waitUntilEventReaches("streamPlaying", 4);
|
user.getEventManager().waitUntilEventReaches("streamPlaying", 4);
|
||||||
checkDockerContainerRunning("openvidu/openvidu-recording", 1);
|
checkDockerContainerRunning("openvidu/openvidu-recording", 1);
|
||||||
|
|
||||||
OV.fetch();
|
OV.fetch();
|
||||||
session = OV.getActiveSessions().get(0);
|
session = OV.getActiveSessions().get(0);
|
||||||
session.close();
|
session.close();
|
||||||
|
|
||||||
event = CustomWebhook.waitForEvent("recordingStatusChanged", 1);
|
// Recording hasn't had time to start. Should trigger stopped, then failed immediately
|
||||||
if ("stopped".equals(event.get("status").getAsString())) {
|
event = CustomWebhook.waitForEvent("recordingStatusChanged", 1); // stopped
|
||||||
// Recording hasn't had time to start. Should trigger stopped, started, failed
|
Assertions.assertEquals("stopped", event.get("status").getAsString(),
|
||||||
event = CustomWebhook.waitForEvent("recordingStatusChanged", 5); // started
|
"Wrong status in recordingStatusChanged event");
|
||||||
Assertions.assertEquals("started", event.get("status").getAsString(),
|
|
||||||
"Wrong status in recordingStatusChanged event");
|
|
||||||
try {
|
|
||||||
// Wait first for a failed event
|
|
||||||
event = CustomWebhook.waitForEvent("recordingStatusChanged", 10); // failed
|
|
||||||
Assertions.assertEquals("failed", event.get("status").getAsString(),
|
|
||||||
"Wrong status in recordingStatusChanged event");
|
|
||||||
Assertions.assertEquals(Recording.Status.failed, OV.getRecording(sessionName + "~2").getStatus(),
|
|
||||||
"Wrong recording status");
|
|
||||||
} catch (Exception e) {
|
|
||||||
// If the failed event is not received, it's because the session has been destroyed
|
|
||||||
// before the recording started
|
|
||||||
// Check for sessionDestroyed event
|
|
||||||
event = CustomWebhook.waitForEvent("sessionDestroyed", 5);
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
// Recording did have time to start. Should trigger started, stopped, ready
|
|
||||||
event = CustomWebhook.waitForEvent("recordingStatusChanged", 5); // started
|
|
||||||
Assertions.assertEquals("stopped", event.get("status").getAsString(),
|
|
||||||
"Wrong status in recordingStatusChanged event");
|
|
||||||
event = CustomWebhook.waitForEvent("recordingStatusChanged", 10); // failed
|
|
||||||
Assertions.assertEquals("ready", event.get("status").getAsString(),
|
|
||||||
"Wrong status in recordingStatusChanged event");
|
|
||||||
Assertions.assertEquals(Recording.Status.ready, OV.getRecording(sessionName + "~2").getStatus(),
|
|
||||||
"Wrong recording status");
|
|
||||||
}
|
|
||||||
|
|
||||||
checkDockerContainerRunning("openvidu/openvidu-recording", 0);
|
event = CustomWebhook.waitForEvent("recordingStatusChanged", 5); // failed
|
||||||
|
Assertions.assertEquals("failed", event.get("status").getAsString(),
|
||||||
|
"Wrong status in recordingStatusChanged event");
|
||||||
|
Assertions.assertEquals(Recording.Status.failed, OV.getRecording(sessionName + "~2").getStatus(),
|
||||||
|
"Wrong recording status");
|
||||||
|
|
||||||
} finally {
|
checkDockerContainerRunning("openvidu/openvidu-recording", 0); } finally {
|
||||||
CustomWebhook.shutDown();
|
CustomWebhook.shutDown();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue