[stable33] feat: improve async signing error handling

lib/BackgroundJob/SignFileJob.php

@@ -28,7 +28,6 @@ public function __construct(
 	 */
 	#[\Override]
 	public function run($argument): void {
-		// Handle null arguments from Nextcloud background job system
 		$argument = is_array($argument) ? $argument : [];
 		$this->coordinator->runSignFile($argument);
 	}

lib/BackgroundJob/SignSingleFileJob.php

@@ -29,7 +29,6 @@ public function __construct(
 	 */
 	#[\Override]
 	public function run($argument): void {
-		// Handle null arguments from Nextcloud background job system
 		$argument = is_array($argument) ? $argument : [];
 		$this->coordinator->runSignSingleFile($argument);
 	}

lib/Controller/FileProgressController.php

@@ -30,6 +30,10 @@
 
 /**
  * @psalm-import-type LibresignValidateFile from \OCA\Libresign\ResponseDefinitions
+ * @psalm-import-type LibresignProgressPayload from \OCA\Libresign\ResponseDefinitions
+ * @psalm-import-type LibresignProgressError from \OCA\Libresign\ResponseDefinitions
+ * @psalm-import-type LibresignProgressResponse from \OCA\Libresign\ResponseDefinitions
+ * @psalm-import-type LibresignProgressFile from \OCA\Libresign\ResponseDefinitions
  */
 class FileProgressController extends AEnvironmentAwareController {
 	public function __construct(
@@ -53,7 +57,7 @@ public function __construct(
 	 *
 	 * @param string $uuid Sign request UUID
 	 * @param int $timeout Maximum seconds to wait (default 30)
-	 * @return DataResponse<Http::STATUS_OK, array{status: string, statusCode: int, statusText: string, fileId: int, progress: array<string, mixed>, file?: LibresignValidateFile}, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string, status: string}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, LibresignProgressResponse, array{}>|DataResponse<Http::STATUS_NOT_FOUND, array{message: string, status: string}, array{}>
 	 *
 	 * 200: Status and progress returned
 	 * 404: Sign request not found
@@ -64,14 +68,17 @@ public function __construct(
 	#[PublicPage]
 	#[ApiRoute(verb: 'GET', url: '/api/{apiVersion}/file/progress/{uuid}', requirements: ['apiVersion' => '(v1)'])]
 	public function checkProgressByUuid(string $uuid, int $timeout = 30): DataResponse {
+		$timeout = max(1, min($timeout, 30));
 		try {
 			$signRequest = $this->signRequestMapper->getByUuid($uuid);
 			$file = $this->fileMapper->getById($signRequest->getFileId());
 			$currentStatus = $this->progressService->getStatusCodeForSignRequest($file, $signRequest);
 
-			if ($file->getStatus() === FileStatus::SIGNING_IN_PROGRESS->value) {
-				$this->workerHealthService->ensureWorkerRunning();
-				$currentStatus = $this->progressService->pollForStatusChange($uuid, $currentStatus, $timeout);
+			if ($timeout > 0) {
+				if ($file->getStatus() === FileStatus::SIGNING_IN_PROGRESS->value) {
+					$this->workerHealthService->ensureWorkerRunning();
+				}
+				$currentStatus = $this->progressService->pollForStatusOrErrorChange($file, $signRequest, $currentStatus, $timeout);
 			}
 
 			return $this->buildStatusResponse($file, $signRequest, $currentStatus);
@@ -90,12 +97,19 @@ public function checkProgressByUuid(string $uuid, int $timeout = 30): DataRespon
 	 * @param FileEntity $file The file entity
 	 * @param SignRequestEntity $signRequest The sign request entity
 	 * @param int $status Current status code
-	 * @return DataResponse<Http::STATUS_OK, array{status: string, statusCode: int, statusText: string, fileId: int, progress: array<string, mixed>, file?: LibresignValidateFile}, array{}>
-	 * @psalm-return DataResponse<Http::STATUS_OK, array{status: string, statusCode: int, statusText: string, fileId: int, progress: array<string, mixed>, file?: LibresignValidateFile}, array{}>
+	 * @return DataResponse<Http::STATUS_OK, LibresignProgressResponse, array{}>
+	 * @psalm-return DataResponse<Http::STATUS_OK, LibresignProgressResponse, array{}>
 	 */
 	private function buildStatusResponse(FileEntity $file, SignRequestEntity $signRequest, int $status): DataResponse {
 		$statusEnum = FileStatus::tryFrom($status);
+		/** @psalm-var LibresignProgressPayload $progress */
 		$progress = $this->progressService->getSignRequestProgress($file, $signRequest);
+		/** @psalm-var LibresignProgressError|null $error */
+		$error = $this->progressService->getSignRequestError($signRequest->getUuid());
+
+		$hasFileErrors = !empty($progress['files']) && $this->hasErrorsInFiles($progress['files']);
+
+		/** @psalm-var LibresignProgressResponse $response */
 		$response = [
 			'status' => $statusEnum?->name ?? 'UNKNOWN',
 			'statusCode' => $status,
@@ -104,7 +118,16 @@ private function buildStatusResponse(FileEntity $file, SignRequestEntity $signRe
 			'progress' => $progress,
 		];
 
-		if ($this->progressService->isProgressComplete($progress)) {
+		if ($error && !$hasFileErrors) {
+			$response['status'] = 'ERROR';
+			if (!empty($error['message'])) {
+				$response['statusText'] = (string)$error['message'];
+			}
+			$response['error'] = $error;
+		}
+
+		$hasAnyError = $error || $hasFileErrors || ($progress['errors'] ?? 0) > 0;
+		if (!$hasAnyError && $this->progressService->isProgressComplete($progress)) {
 			$response['file'] = $this->fileService
 				->setFile($file)
 				->setSignRequest($signRequest)
@@ -121,4 +144,16 @@ private function buildStatusResponse(FileEntity $file, SignRequestEntity $signRe
 
 		return new DataResponse($response, Http::STATUS_OK);
 	}
+
+	/**
+	 * @param list<LibresignProgressFile> $files
+	 */
+	private function hasErrorsInFiles(array $files): bool {
+		foreach ($files as $file) {
+			if (!empty($file['error'])) {
+				return true;
+			}
+		}
+		return false;
+	}
 }

lib/Db/SignRequestMapper.php

@@ -124,6 +124,22 @@ public function getByUuid(string $uuid): SignRequest {
 		return $signRequest;
 	}
 
+	/**
+	 * @throws DoesNotExistException
+	 */
+	public function getByUuidUncached(string $uuid): SignRequest {
+		$qb = $this->db->getQueryBuilder();
+
+		$qb->select('*')
+			->from($this->getTableName())
+			->where(
+				$qb->expr()->eq('uuid', $qb->createNamedParameter($uuid))
+			);
+
+		/** @var SignRequest */
+		return $this->findEntity($qb);
+	}
+
 	public function getByEmailAndFileId(string $email, int $fileId): SignRequest {
 		$qb = $this->db->getQueryBuilder();
 

lib/ResponseDefinitions.php

@@ -242,6 +242,44 @@
  *     }[],
  *     visibleElements?: LibresignVisibleElement[],
  * }
+ * @psalm-type LibresignProgressError = array{
+ *     message: string,
+ *     code?: int,
+ *     timestamp?: string,
+ *     fileId?: int,
+ *     signRequestId?: int,
+ *     signRequestUuid?: string,
+ * }
+ * @psalm-type LibresignProgressFile = array{
+ *     id: int,
+ *     name: string,
+ *     status: int,
+ *     statusText: string,
+ *     error?: LibresignProgressError,
+ * }
+ * @psalm-type LibresignProgressPayload = array{
+ *     total: int,
+ *     signed: int,
+ *     inProgress: int,
+ *     pending: int,
+ *     errors?: int,
+ *     files?: list<LibresignProgressFile>,
+ *     signers?: list<array{
+ *         id: int,
+ *         displayName: string,
+ *         signed: ?string,
+ *         status: int,
+ *     }>,
+ * }
+ * @psalm-type LibresignProgressResponse = array{
+ *     status: string,
+ *     statusCode: int,
+ *     statusText: string,
+ *     fileId: int,
+ *     progress: LibresignProgressPayload,
+ *     file?: LibresignValidateFile,
+ *     error?: LibresignProgressError,
+ * }
  * @psalm-type LibresignFileListItem = array{
  *     fileId: int,
  *     id: int,

lib/Service/AsyncSigningService.php

@@ -49,6 +49,7 @@ public function enqueueSigningJob(
 		$this->jobList->add(SignFileJob::class, [
 			'fileId' => $libreSignFile->getId(),
 			'signRequestId' => $signRequest->getId(),
+			'signRequestUuid' => $signRequest->getUuid(),
 			'userId' => $user?->getUID(),
 			'credentialsId' => $credentialsId,
 			'userUniqueIdentifier' => $userUniqueIdentifier,

lib/Service/SignFileService.php

@@ -42,6 +42,7 @@
 use OCA\Libresign\Service\Envelope\EnvelopeStatusDeterminer;
 use OCA\Libresign\Service\IdentifyMethod\IIdentifyMethod;
 use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\IToken;
+use OCA\Libresign\Service\SignRequest\StatusService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -52,8 +53,6 @@
 use OCP\Files\NotPermittedException;
 use OCP\Http\Client\IClientService;
 use OCP\IAppConfig;
-use OCP\ICache;
-use OCP\ICacheFactory;
 use OCP\IDateTimeZone;
 use OCP\IL10N;
 use OCP\ITempManager;
@@ -81,7 +80,6 @@ class SignFileService {
 	private string $friendlyName = '';
 	private ?IUser $user = null;
 	private ?SignEngineHandler $engine = null;
-	private ICache $cache;
 	private PfxProvider $pfxProvider;
 
 	public function __construct(
@@ -116,14 +114,13 @@ public function __construct(
 		private PdfSignatureDetectionService $pdfSignatureDetectionService,
 		private SequentialSigningService $sequentialSigningService,
 		private FileStatusService $fileStatusService,
+		private StatusService $statusService,
 		private IJobList $jobList,
 		private ICredentialsManager $credentialsManager,
 		private EnvelopeStatusDeterminer $envelopeStatusDeterminer,
 		private TsaValidationService $tsaValidationService,
-		ICacheFactory $cacheFactory,
 		PfxProvider $pfxProvider,
 	) {
-		$this->cache = $cacheFactory->createDistributed('libresign_progress');
 		$this->pfxProvider = $pfxProvider;
 	}
 
@@ -550,6 +547,7 @@ private function enqueueSigningJobForFile(SignRequestEntity $signRequest, FileEn
 		$args = array_merge($args, [
 			'fileId' => $file->getId(),
 			'signRequestId' => $signRequest->getId(),
+			'signRequestUuid' => $signRequest->getUuid(),
 		]);
 
 		$this->jobList->add(SignSingleFileJob::class, $args);
@@ -972,15 +970,11 @@ protected function setNewStatusIfNecessary(FileEntity $libreSignFile): bool {
 	}
 
 	private function updateCacheAfterDbSave(FileEntity $libreSignFile): void {
-		$cacheKey = 'status_' . $libreSignFile->getUuid();
-		$status = $libreSignFile->getStatus();
-		$this->cache->set($cacheKey, $status, 60); // Cache for 60 seconds
+		$this->statusService->cacheFileStatus($libreSignFile);
 	}
 
 	private function updateEntityCacheAfterDbSave(FileEntity $file): void {
-		$cacheKey = 'status_' . $file->getUuid();
-		$status = $file->getStatus();
-		$this->cache->set($cacheKey, $status, 60);
+		$this->statusService->cacheFileStatus($file);
 	}
 
 	private function evaluateStatusFromSigners(): ?int {

lib/Service/SignJobCoordinator.php

@@ -15,6 +15,8 @@
 use OCA\Libresign\Db\SignRequest as SignRequestEntity;
 use OCA\Libresign\Db\SignRequestMapper;
 use OCA\Libresign\Enum\FileStatus;
+use OCA\Libresign\Service\SignRequest\Error\SignRequestErrorReporter;
+use OCA\Libresign\Service\SignRequest\ProgressService;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Security\ICredentialsManager;
@@ -28,18 +30,23 @@ public function __construct(
 		private FolderService $folderService,
 		private IUserManager $userManager,
 		private ICredentialsManager $credentialsManager,
+		private ProgressService $progressService,
+		private SignRequestErrorReporter $signRequestErrorReporter,
 		private LoggerInterface $logger,
 	) {
 	}
 
 	public function runSignFile(array $argument): void {
+		$file = null;
+		$signRequest = null;
 		try {
 			if (empty($argument)) {
 				throw new \InvalidArgumentException('SignFileJob: Cannot proceed with empty arguments');
 			}
 
 			[$fileId, $signRequestId] = $this->requireIds($argument, 'SignFileJob');
 			[$file, $signRequest] = $this->loadEntities($fileId, $signRequestId);
+			$this->progressService->clearSignRequestError($signRequest->getUuid());
 			$user = $this->resolveUser($argument['userId'] ?? null, null, null);
 			if ($user) {
 				$this->folderService->setUserId($user->getUID());
@@ -50,9 +57,11 @@ public function runSignFile(array $argument): void {
 
 			$this->signFileService->sign();
 		} catch (\Throwable $e) {
-			$this->logger->error('SignFileJob failed: {error}', [
-				'error' => $e->getMessage(),
+			$this->signRequestErrorReporter->error('SignFileJob failed', [
 				'exception' => $e,
+				'fileId' => $file?->getId() ?? ($argument['fileId'] ?? null),
+				'signRequestId' => $signRequest?->getId() ?? ($argument['signRequestId'] ?? null),
+				'signRequestUuid' => $signRequest?->getUuid() ?? ($argument['signRequestUuid'] ?? null),
 			]);
 		} finally {
 			$this->deleteCredentials($argument['userId'] ?? '', $argument['credentialsId'] ?? null);
@@ -73,7 +82,7 @@ public function runSignSingleFile(array $argument): void {
 
 			[$fileId, $signRequestId] = $this->requireIds($argument, 'SignSingleFileJob');
 			[$file, $signRequest] = $this->loadEntities($fileId, $signRequestId);
-
+			$this->progressService->clearSignRequestError($signRequest->getUuid());
 
 			$effectiveUserId = $effectiveUserId
 				?? $file->getUserId()
@@ -90,11 +99,11 @@ public function runSignSingleFile(array $argument): void {
 
 			$this->signFileService->signSingleFile($file, $signRequest);
 		} catch (\Throwable $e) {
-			$contextFileId = $fileId ?? ($argument['fileId'] ?? 'unknown');
-			$this->logger->error('SignSingleFileJob failed for file {fileId}: {error}', [
-				'fileId' => $contextFileId,
-				'error' => $e->getMessage(),
+			$this->signRequestErrorReporter->error('SignSingleFileJob failed for file {fileId}', [
 				'exception' => $e,
+				'fileId' => $file?->getId() ?? ($argument['fileId'] ?? null),
+				'signRequestId' => $signRequest?->getId() ?? ($argument['signRequestId'] ?? null),
+				'signRequestUuid' => $signRequest?->getUuid() ?? ($argument['signRequestUuid'] ?? null),
 			]);
 		} finally {
 			$this->deleteCredentials($argument['userId'] ?? ($effectiveUserId ?? ''), $argument['credentialsId'] ?? null);