Fix editor stuck on "Loading document" when appdata is on local storage

lib/Controller/ConvertController.php

@@ -23,21 +23,31 @@
 
 namespace OCA\DocumentServer\Controller;
 
+use OCA\DocumentServer\Document\DocumentConversionException;
 use OCA\DocumentServer\Document\DocumentStore;
+use OCA\DocumentServer\Document\PasswordRequiredException;
 use OCA\DocumentServer\OnlyOffice\URLDecoder;
 use OCA\DocumentServer\JSONResponse;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
 use OCP\AppFramework\Http\Attribute\PublicPage;
+use OCP\IRequest;
 use OCP\IURLGenerator;
 
 class ConvertController extends Controller {
 	private $documentStore;
 	private $urlDecoder;
 	private $urlGenerator;
 
-	public function __construct(DocumentStore $documentStore, URLDecoder $urlDecoder, IURLGenerator $urlGenerator) {
+	public function __construct(
+		string $appName,
+		IRequest $request,
+		DocumentStore $documentStore,
+		URLDecoder $urlDecoder,
+		IURLGenerator $urlGenerator
+	) {
+		parent::__construct($appName, $request);
 		$this->documentStore = $documentStore;
 		$this->urlDecoder = $urlDecoder;
 		$this->urlGenerator = $urlGenerator;
@@ -57,8 +67,22 @@ public function convert(bool $async, string $url, string $outputtype, string $fi
 		} else {
 			$documentId = (int)$key;
 			$documentFile = $this->urlDecoder->getFileForUrl($url);
-			$this->documentStore->getDocumentForEditor($documentId, $documentFile, $filetype);
-			$this->documentStore->convert($documentId, $outputtype);
+
+			// A failed conversion must come back as a structured {"error": <code>}
+			// body, not as an uncaught exception: this is a plain HTTP endpoint
+			// with no other handler, so without this catch the converter's
+			// exception escapes as a 500 the caller can't interpret. This endpoint
+			// answers the connector over the server-side conversion API, whose
+			// error codes differ from the editor's c_oAscError space used in the
+			// socket replies: -5 incorrect password, -3 conversion error.
+			try {
+				$this->documentStore->getDocumentForEditor($documentId, $documentFile, $filetype);
+				$this->documentStore->convert($documentId, $outputtype);
+			} catch (PasswordRequiredException $e) {
+				return new JSONResponse(['error' => -5]);
+			} catch (DocumentConversionException $e) {
+				return new JSONResponse(['error' => -3]);
+			}
 
 			$url = $this->urlGenerator->linkToRouteAbsolute(
 				'documentserver_community.Document.documentFile', [

lib/Controller/DocumentController.php

@@ -187,7 +187,47 @@ public function upload(int $docId, string $index) {
 	public function download(int $docId, string $cmd) {
 		$cmd = json_decode($cmd, true);
 		$content = fopen('php://input', 'r');
-		$title = $this->documentStore->convertForDownload($docId, $content, $cmd);
+
+		// A failed save-as conversion must be reported to the editor as a 'save'
+		// error, not escape as an uncaught 500: this endpoint has no other
+		// handler, and the editor is waiting on a documentOpen 'save' reply. Tell
+		// the session it failed (-4 = sdkjs DownloadError) so the user sees a
+		// failed download instead of a silent hang, then return the same error to
+		// the HTTP caller. The reply push is itself guarded: if the IPC backend is
+		// what failed, getChannel() would throw and re-raise an uncaught 500.
+		try {
+			$title = $this->documentStore->convertForDownload($docId, $content, $cmd);
+		} catch (\Exception $e) {
+			$this->logger->warning('documentserver download conversion error: {error}', [
+				'error' => $e->getMessage(),
+				'exception' => $e,
+			]);
+
+			try {
+				$session = $this->sessionManager->getSessionForUser($cmd['userconnectionid']);
+				if ($session) {
+					$this->ipcFactory->getChannel($session->getSessionId())->pushMessage(json_encode([
+						'type' => 'documentOpen',
+						'data' => [
+							'type' => 'save',
+							'status' => 'err',
+							'data' => -4,
+						],
+					]));
+				}
+			} catch (\Exception $replyError) {
+				$this->logger->warning('documentserver download could not deliver error reply: {error}', [
+					'error' => $replyError->getMessage(),
+					'exception' => $replyError,
+				]);
+			}
+
+			return new DataResponse([
+				'type' => 'save',
+				'status' => 'err',
+				'data' => -4,
+			]);
+		}
 
 		$session = $this->sessionManager->getSessionForUser($cmd['userconnectionid']);
 		if ($session) {
@@ -361,6 +401,35 @@ private function handleEngineIOPacket(string $packet, string $sid, string $docum
 					'error' => $e->getMessage(),
 					'exception' => $e,
 				]);
+
+				// Don't let a failed command (e.g. the document failing to
+				// open) disappear into the log: with no reply the editor stays
+				// on "Loading document" forever. Push a documentOpen error so
+				// the client surfaces it instead of spinning. Same message shape
+				// the client already handles for the password case in
+				// OpenDocument::openDocument; sdkjs parses data as a
+				// c_oAscError.ID and shows the matching error, -4 is
+				// DownloadError ("download failed"), the closest generic
+				// open-failure code. Guard the reply itself: if the original
+				// failure was the IPC backend being unavailable, getChannel()
+				// throws too, and an unguarded re-throw here would turn the
+				// swallowed error back into an uncaught 500.
+				try {
+					$sessionChannel = $this->ipcFactory->getChannel($sid);
+					$sessionChannel->pushMessage(json_encode([
+						'type' => 'documentOpen',
+						'data' => [
+							'type' => 'open',
+							'status' => 'err',
+							'data' => -4,
+						],
+					]));
+				} catch (\Exception $replyError) {
+					$this->logger->warning('documentserver socketIO could not deliver error reply: {error}', [
+						'error' => $replyError->getMessage(),
+						'exception' => $replyError,
+					]);
+				}
 			}
 		}
 		// sio types '0' (CONNECT) and '1' (DISCONNECT) require no action here.

lib/Document/ConverterBinary.php

@@ -53,6 +53,14 @@ public function run(string $param, string $password = null): string {
 		}
 		$process = proc_open($cmd, $descriptorSpec, $pipes, self::BINARY_DIRECTORY, ["LD_LIBRARY_PATH" => "."]);
 
+		// proc_open returns false if x2t couldn't be spawned at all (missing
+		// binary, fork failure). Without this the next lines fclose/read null
+		// pipes and proc_close(false) warns, and the caller silently gets no
+		// output instead of a clear failure.
+		if ($process === false) {
+			throw new DocumentConversionException("failed to start x2t");
+		}
+
 		@fclose($pipes[0]);
 		$output = @stream_get_contents($pipes[1]);
 		$error = @stream_get_contents($pipes[2]);
@@ -65,9 +73,18 @@ public function run(string $param, string $password = null): string {
 
 		if ($error) {
 			throw new DocumentConversionException($error);
-		} else {
-			return $output;
 		}
+
+		// x2t can fail with a nonzero exit status while writing nothing to stderr;
+		// without this the caller treats a failed conversion as success and later
+		// trips over the missing Editor.bin, see #70. Stderr is checked first so its
+		// (more specific) message wins, including the "Empty sFileFrom or sFileTo"
+		// string that test() relies on.
+		if ($status !== 0) {
+			throw new DocumentConversionException("x2t exited with status $status");
+		}
+
+		return $output;
 	}
 
 	public function test(): bool {

lib/LocalAppData.php

@@ -23,13 +23,16 @@
 
 namespace OCA\DocumentServer;
 
+use OCP\Files\Cache\IScanner;
 use OCP\Files\Folder;
 use OCP\Files\File;
 use OCP\Files\IAppData;
 use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFolder;
 use OCP\IConfig;
 use OCP\ITempManager;
+use OCP\Lock\LockedException;
+use Psr\Log\LoggerInterface;
 
 /**
  * Provide local access to appdata folders
@@ -38,15 +41,18 @@ class LocalAppData {
 	private $appData;
 	private $config;
 	private $tmpManager;
+	private LoggerInterface $logger;
 
 	public function __construct(
 		IAppData $appData,
 		IConfig $config,
-		ITempManager $tmpManager
+		ITempManager $tmpManager,
+		LoggerInterface $logger
 	) {
 		$this->appData = $appData;
 		$this->config = $config;
 		$this->tmpManager = $tmpManager;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -100,6 +106,33 @@ public function getReadWriteLocalPath(ISimpleFolder $folder, callable $callback)
 
 		if (is_dir($localPath)) {
 			$callback($localPath);
+			// The callback (e.g. x2t producing Editor.bin) writes straight to the
+			// on-disk appdata folder, bypassing the Nextcloud storage layer, so the
+			// file cache never learns about the new files and a later getFile() would
+			// throw NotFoundException. Rescan the folder so the writes become visible,
+			// see #70 (editor stuck on "Loading document" because Editor.bin is on disk
+			// but absent from oc_filecache). The temp-folder branch below does not need
+			// this because copyFromLocal() writes back through File::putContent(),
+			// which updates the cache itself. The scan must be RECURSIVE: x2t emits
+			// not just Editor.bin but a media/ subfolder of embedded images, and
+			// the open flow reads those back through the file API
+			// (DocumentStore::getEmbeddedFiles -> media/.getDirectoryListing()). A
+			// shallow scan would register Editor.bin but leave media/'s children
+			// uncached, so embedded images would silently fail to load. A
+			// concurrent writer can hold the scan lock; mirror NC core's
+			// backgroundScan and skip rather than fail the open, the next access
+			// rescans.
+			try {
+				$fullFolder->getStorage()->getScanner()->scan(
+					$fullFolder->getInternalPath(),
+					IScanner::SCAN_RECURSIVE
+				);
+			} catch (LockedException $e) {
+				$this->logger->warning(
+					'documentserver: appdata rescan after conversion skipped, folder locked',
+					['exception' => $e, 'app' => 'documentserver_community']
+				);
+			}
 		} else {
 			$localPath = $this->tmpManager->getTemporaryFolder();
 			$this->copyToLocal($fullFolder, $localPath);