mirror of
https://github.com/nextcloud/server.git
synced 2026-05-28 04:32:30 -04:00
feat(TaskProcessing): user-facing error messages
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
This commit is contained in:
parent
ea8ab8e192
commit
7373f11af6
7 changed files with 113 additions and 16 deletions
|
|
@ -460,6 +460,7 @@ class TaskProcessingApiController extends OCSController {
|
|||
* @param int $taskId The id of the task
|
||||
* @param array<string,mixed>|null $output The resulting task output, files are represented by their IDs
|
||||
* @param string|null $errorMessage An error message if the task failed
|
||||
* @param string|null $userFacingErrorMessage An error message that will be shown to the user
|
||||
* @return DataResponse<Http::STATUS_OK, array{task: CoreTaskProcessingTask}, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
|
||||
*
|
||||
* 200: Result updated successfully
|
||||
|
|
@ -467,10 +468,10 @@ class TaskProcessingApiController extends OCSController {
|
|||
*/
|
||||
#[ExAppRequired]
|
||||
#[ApiRoute(verb: 'POST', url: '/tasks_provider/{taskId}/result', root: '/taskprocessing')]
|
||||
public function setResult(int $taskId, ?array $output = null, ?string $errorMessage = null): DataResponse {
|
||||
public function setResult(int $taskId, ?array $output = null, ?string $errorMessage = null, ?string $userFacingErrorMessage = null): DataResponse {
|
||||
try {
|
||||
// set result
|
||||
$this->taskProcessingManager->setTaskResult($taskId, $errorMessage, $output, true);
|
||||
$this->taskProcessingManager->setTaskResult($taskId, $errorMessage, $output, isUsingFileIds: true, userFacingError: $userFacingErrorMessage);
|
||||
$task = $this->taskProcessingManager->getTask($taskId);
|
||||
|
||||
/** @var CoreTaskProcessingTask $json */
|
||||
|
|
|
|||
48
core/Migrations/Version33000Date20251013110519.php
Normal file
48
core/Migrations/Version33000Date20251013110519.php
Normal file
|
|
@ -0,0 +1,48 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
|
||||
* SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
*/
|
||||
namespace OC\Core\Migrations;
|
||||
|
||||
use Closure;
|
||||
use OCP\DB\ISchemaWrapper;
|
||||
use OCP\DB\Types;
|
||||
use OCP\Migration\Attributes\AddColumn;
|
||||
use OCP\Migration\Attributes\ColumnType;
|
||||
use OCP\Migration\IOutput;
|
||||
use OCP\Migration\SimpleMigrationStep;
|
||||
|
||||
/**
|
||||
*
|
||||
*/
|
||||
#[AddColumn(table: 'taskprocessing_tasks', name: 'user_facing_error_message', type: ColumnType::STRING)]
|
||||
class Version33000Date20251013110519 extends SimpleMigrationStep {
|
||||
|
||||
/**
|
||||
* @param IOutput $output
|
||||
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
|
||||
* @param array $options
|
||||
* @return null|ISchemaWrapper
|
||||
*/
|
||||
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
|
||||
/** @var ISchemaWrapper $schema */
|
||||
$schema = $schemaClosure();
|
||||
|
||||
if ($schema->hasTable('taskprocessing_tasks')) {
|
||||
$table = $schema->getTable('taskprocessing_tasks');
|
||||
if (!$table->hasColumn('user_facing_error_message')) {
|
||||
$table->addColumn('user_facing_error_message', Types::STRING, [
|
||||
'notnull' => false,
|
||||
'length' => 4000,
|
||||
]);
|
||||
return $schema;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
|
@ -47,6 +47,8 @@ use OCP\TaskProcessing\Task as OCPTask;
|
|||
* @method int getEndedAt()
|
||||
* @method setAllowCleanup(int $allowCleanup)
|
||||
* @method int getAllowCleanup()
|
||||
* @method setUserFacingErrorMessage(null|string $message)
|
||||
* @method null|string getUserFacingErrorMessage()
|
||||
*/
|
||||
class Task extends Entity {
|
||||
protected $lastUpdated;
|
||||
|
|
@ -66,16 +68,17 @@ class Task extends Entity {
|
|||
protected $startedAt;
|
||||
protected $endedAt;
|
||||
protected $allowCleanup;
|
||||
protected $userFacingErrorMessage;
|
||||
|
||||
/**
|
||||
* @var string[]
|
||||
*/
|
||||
public static array $columns = ['id', 'last_updated', 'type', 'input', 'output', 'status', 'user_id', 'app_id', 'custom_id', 'completion_expected_at', 'error_message', 'progress', 'webhook_uri', 'webhook_method', 'scheduled_at', 'started_at', 'ended_at', 'allow_cleanup'];
|
||||
public static array $columns = ['id', 'last_updated', 'type', 'input', 'output', 'status', 'user_id', 'app_id', 'custom_id', 'completion_expected_at', 'error_message', 'progress', 'webhook_uri', 'webhook_method', 'scheduled_at', 'started_at', 'ended_at', 'allow_cleanup', 'user_facing_error_message'];
|
||||
|
||||
/**
|
||||
* @var string[]
|
||||
*/
|
||||
public static array $fields = ['id', 'lastUpdated', 'type', 'input', 'output', 'status', 'userId', 'appId', 'customId', 'completionExpectedAt', 'errorMessage', 'progress', 'webhookUri', 'webhookMethod', 'scheduledAt', 'startedAt', 'endedAt', 'allowCleanup'];
|
||||
public static array $fields = ['id', 'lastUpdated', 'type', 'input', 'output', 'status', 'userId', 'appId', 'customId', 'completionExpectedAt', 'errorMessage', 'progress', 'webhookUri', 'webhookMethod', 'scheduledAt', 'startedAt', 'endedAt', 'allowCleanup', 'userFacingErrorMessage'];
|
||||
|
||||
|
||||
public function __construct() {
|
||||
|
|
@ -98,6 +101,7 @@ class Task extends Entity {
|
|||
$this->addType('startedAt', 'integer');
|
||||
$this->addType('endedAt', 'integer');
|
||||
$this->addType('allowCleanup', 'integer');
|
||||
$this->addType('userFacingErrorMessage', 'string');
|
||||
}
|
||||
|
||||
public function toRow(): array {
|
||||
|
|
@ -127,6 +131,7 @@ class Task extends Entity {
|
|||
'startedAt' => $task->getStartedAt(),
|
||||
'endedAt' => $task->getEndedAt(),
|
||||
'allowCleanup' => $task->getAllowCleanup() ? 1 : 0,
|
||||
'userFacingErrorMessage' => $task->getUserFacingErrorMessage(),
|
||||
]);
|
||||
return $taskEntity;
|
||||
}
|
||||
|
|
@ -150,6 +155,7 @@ class Task extends Entity {
|
|||
$task->setStartedAt($this->getStartedAt());
|
||||
$task->setEndedAt($this->getEndedAt());
|
||||
$task->setAllowCleanup($this->getAllowCleanup() !== 0);
|
||||
$task->setUserFacingErrorMessage($this->getUserFacingErrorMessage());
|
||||
return $task;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1018,7 +1018,7 @@ class Manager implements IManager {
|
|||
$output = $provider->process($task->getUserId(), $input, fn (float $progress) => $this->setTaskProgress($task->getId(), $progress));
|
||||
} catch (ProcessingException $e) {
|
||||
$this->logger->warning('Failed to process a TaskProcessing task with synchronous provider ' . $provider->getId(), ['exception' => $e]);
|
||||
$this->setTaskResult($task->getId(), $e->getMessage(), null);
|
||||
$this->setTaskResult($task->getId(), $e->getMessage(), null, userFacingError: $e->getUserFacingMessage());
|
||||
return false;
|
||||
} catch (\Throwable $e) {
|
||||
$this->logger->error('Unknown error while processing TaskProcessing task', ['exception' => $e]);
|
||||
|
|
@ -1089,7 +1089,7 @@ class Manager implements IManager {
|
|||
return true;
|
||||
}
|
||||
|
||||
public function setTaskResult(int $id, ?string $error, ?array $result, bool $isUsingFileIds = false): void {
|
||||
public function setTaskResult(int $id, ?string $error, ?array $result, bool $isUsingFileIds = false, ?string $userFacingError = null): void {
|
||||
// TODO: Not sure if we should rather catch the exceptions of getTask here and fail silently
|
||||
$task = $this->getTask($id);
|
||||
if ($task->getStatus() === Task::STATUS_CANCELLED) {
|
||||
|
|
@ -1101,6 +1101,8 @@ class Manager implements IManager {
|
|||
$task->setEndedAt(time());
|
||||
// truncate error message to 1000 characters
|
||||
$task->setErrorMessage(mb_substr($error, 0, 1000));
|
||||
// truncate error message to 1000 characters
|
||||
$task->setUserFacingErrorMessage(mb_substr($userFacingError, 0, 1000));
|
||||
$this->logger->warning('A TaskProcessing ' . $task->getTaskTypeId() . ' task with id ' . $id . ' failed with the following message: ' . $error);
|
||||
} elseif ($result !== null) {
|
||||
$taskTypes = $this->getAvailableTaskTypes();
|
||||
|
|
|
|||
|
|
@ -16,4 +16,21 @@ namespace OCP\TaskProcessing\Exception;
|
|||
* @since 30.0.0
|
||||
*/
|
||||
class ProcessingException extends \RuntimeException {
|
||||
private string $userFacingMessage;
|
||||
|
||||
/**
|
||||
* @since 33.0.0
|
||||
*/
|
||||
public function getUserFacingMessage(): string {
|
||||
return $this->userFacingMessage;
|
||||
}
|
||||
|
||||
/**
|
||||
* @since 33.0.0
|
||||
*/
|
||||
public function setUserFacingMessage(string $userFacingMessage): void {
|
||||
$this->userFacingMessage = $userFacingMessage;
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -133,11 +133,13 @@ interface IManager {
|
|||
* @param string|null $error
|
||||
* @param array|null $result
|
||||
* @param bool $isUsingFileIds
|
||||
* @param string|null $userFacingError
|
||||
* @throws Exception If the query failed
|
||||
* @throws NotFoundException If the task could not be found
|
||||
* @since 30.0.0
|
||||
* @aince 33.0.0 Added `userFacingError` parameter
|
||||
*/
|
||||
public function setTaskResult(int $id, ?string $error, ?array $result, bool $isUsingFileIds = false): void;
|
||||
public function setTaskResult(int $id, ?string $error, ?array $result, bool $isUsingFileIds = false, ?string $userFacingError = null): void;
|
||||
|
||||
/**
|
||||
* @param int $id
|
||||
|
|
|
|||
|
|
@ -31,8 +31,24 @@ final class Task implements \JsonSerializable {
|
|||
protected int $lastUpdated;
|
||||
|
||||
protected ?string $webhookUri = null;
|
||||
|
||||
protected ?string $webhookMethod = null;
|
||||
|
||||
/**
|
||||
* @psalm-var self::STATUS_*
|
||||
*/
|
||||
protected int $status = self::STATUS_UNKNOWN;
|
||||
|
||||
protected ?int $scheduledAt = null;
|
||||
|
||||
protected ?int $startedAt = null;
|
||||
|
||||
protected ?int $endedAt = null;
|
||||
|
||||
protected bool $allowCleanup = true;
|
||||
|
||||
protected ?string $userFacingErrorMessage = null;
|
||||
|
||||
/**
|
||||
* @since 30.0.0
|
||||
*/
|
||||
|
|
@ -58,15 +74,6 @@ final class Task implements \JsonSerializable {
|
|||
*/
|
||||
public const STATUS_UNKNOWN = 0;
|
||||
|
||||
/**
|
||||
* @psalm-var self::STATUS_*
|
||||
*/
|
||||
protected int $status = self::STATUS_UNKNOWN;
|
||||
|
||||
protected ?int $scheduledAt = null;
|
||||
protected ?int $startedAt = null;
|
||||
protected ?int $endedAt = null;
|
||||
protected bool $allowCleanup = true;
|
||||
|
||||
/**
|
||||
* @param string $taskTypeId
|
||||
|
|
@ -389,4 +396,18 @@ final class Task implements \JsonSerializable {
|
|||
default => 'STATUS_UNKNOWN',
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* @since 33.0.0
|
||||
*/
|
||||
public function setUserFacingErrorMessage(?string $userFacingErrorMessage): void {
|
||||
$this->userFacingErrorMessage = $userFacingErrorMessage;
|
||||
}
|
||||
|
||||
/**
|
||||
* @since 33.0.0
|
||||
*/
|
||||
public function getUserFacingErrorMessage(): ?string {
|
||||
return $this->userFacingErrorMessage;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue