Compare commits

...

5 Commits

Author SHA1 Message Date
Marcel Klehr
9eff3be693 fix(settings): Improve TaskProcessingPickupSpeed setup check
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
2025-10-13 14:06:31 +02:00
Marcel Klehr
aa296f5f64 fix(TaskProcessing): Update autoloaders
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
2025-10-13 13:23:50 +02:00
Marcel Klehr
7d2a1568e8 fix(TaskProcessing): Update openapi descriptions for user-facing error messages
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
2025-10-13 13:19:47 +02:00
Marcel Klehr
aaadbdf765 test(TaskProcessing): Add test for user-facing error messages
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
2025-10-13 13:16:19 +02:00
Marcel Klehr
1152dcc3ea feat(TaskProcessing): user-facing error messages
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
2025-10-13 12:39:02 +02:00
14 changed files with 258 additions and 22 deletions

View File

@@ -17,7 +17,8 @@ use OCP\TaskProcessing\IManager;
class TaskProcessingPickupSpeed implements ISetupCheck {
public const MAX_SLOW_PERCENTAGE = 0.2;
public const TIME_SPAN = 24;
public const MAX_DAYS = 14;
public function __construct(
private IL10N $l10n,
@@ -35,10 +36,21 @@ class TaskProcessingPickupSpeed implements ISetupCheck {
}
public function run(): SetupResult {
$tasks = $this->taskProcessingManager->getTasks(userId: '', scheduleAfter: $this->timeFactory->now()->getTimestamp() - 60 * 60 * self::TIME_SPAN); // userId: '' means no filter, whereas null would mean guest
$taskCount = count($tasks);
$taskCount = 0;
$lastNDays = 1;
while($taskCount === 0 && $lastNDays < self::MAX_DAYS) {
$lastNDays++;
$tasks = $this->taskProcessingManager->getTasks(userId: '', scheduleAfter: $this->timeFactory->now()->getTimestamp() - 60 * 60 * 24 * $lastNDays); // userId: '' means no filter, whereas null would mean guest
$taskCount = count($tasks);
}
if ($taskCount === 0) {
return SetupResult::success($this->l10n->n('No scheduled tasks in the last %n hour.', 'No scheduled tasks in the last %n hours.', self::TIME_SPAN));
return SetupResult::success(
$this->l10n->n(
'No scheduled tasks in the last %n hour.',
'No scheduled tasks in the last %n hours.',
24 * $lastNDays
)
);
}
$slowCount = 0;
foreach ($tasks as $task) {
@@ -55,9 +67,22 @@ class TaskProcessingPickupSpeed implements ISetupCheck {
}
if ($slowCount / $taskCount < self::MAX_SLOW_PERCENTAGE) {
return SetupResult::success($this->l10n->n('The task pickup speed has been ok in the last %n hour.', 'The task pickup speed has been ok in the last %n hours.', self::TIME_SPAN));
return SetupResult::success(
$this->l10n->n(
'The task pickup speed has been ok in the last %n hour.',
'The task pickup speed has been ok in the last %n hours.',
24 * $lastNDays
)
);
} else {
return SetupResult::warning($this->l10n->n('The task pickup speed has been slow in the last %n hour. Many tasks took longer than 4 minutes to be picked up. Consider setting up a worker to process tasks in the background.', 'The task pickup speed has been slow in the last %n hours. Many tasks took longer than 4 minutes to be picked up. Consider setting up a worker to process tasks in the background.', self::TIME_SPAN), 'https://docs.nextcloud.com/server/latest/admin_manual/ai/overview.html#improve-ai-task-pickup-speed');
return SetupResult::warning(
$this->l10n->n(
'The task pickup speed has been slow in the last %n hour. Many tasks took longer than 4 minutes to be picked up. Consider setting up a worker to process tasks in the background.',
'The task pickup speed has been slow in the last %n hours. Many tasks took longer than 4 minutes to be picked up. Consider setting up a worker to process tasks in the background.',
24 * $lastNDays
),
'https://docs.nextcloud.com/server/latest/admin_manual/ai/overview.html#improve-ai-task-pickup-speed'
);
}
}
}

View File

@@ -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 */

View 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;
}
}

View File

@@ -960,6 +960,12 @@
"nullable": true,
"default": null,
"description": "An error message if the task failed"
},
"userFacingErrorMessage": {
"type": "string",
"nullable": true,
"default": null,
"description": "An error message that will be shown to the user"
}
}
}

View File

@@ -10865,6 +10865,12 @@
"nullable": true,
"default": null,
"description": "An error message if the task failed"
},
"userFacingErrorMessage": {
"type": "string",
"nullable": true,
"default": null,
"description": "An error message that will be shown to the user"
}
}
}

View File

@@ -1528,6 +1528,7 @@ return array(
'OC\\Core\\Migrations\\Version32000Date20250620081925' => $baseDir . '/core/Migrations/Version32000Date20250620081925.php',
'OC\\Core\\Migrations\\Version32000Date20250731062008' => $baseDir . '/core/Migrations/Version32000Date20250731062008.php',
'OC\\Core\\Migrations\\Version32000Date20250806110519' => $baseDir . '/core/Migrations/Version32000Date20250806110519.php',
'OC\\Core\\Migrations\\Version33000Date20251013110519' => $baseDir . '/core/Migrations/Version33000Date20251013110519.php',
'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php',
'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',

View File

@@ -1569,6 +1569,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Core\\Migrations\\Version32000Date20250620081925' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250620081925.php',
'OC\\Core\\Migrations\\Version32000Date20250731062008' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250731062008.php',
'OC\\Core\\Migrations\\Version32000Date20250806110519' => __DIR__ . '/../../..' . '/core/Migrations/Version32000Date20250806110519.php',
'OC\\Core\\Migrations\\Version33000Date20251013110519' => __DIR__ . '/../../..' . '/core/Migrations/Version33000Date20251013110519.php',
'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php',
'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',

View File

@@ -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;
}
}

View File

@@ -1016,7 +1016,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]);
@@ -1087,7 +1087,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) {
@@ -1099,6 +1099,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();

View File

@@ -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;
}
}

View File

@@ -132,11 +132,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

View File

@@ -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;
}
}

View File

@@ -14381,6 +14381,12 @@
"nullable": true,
"default": null,
"description": "An error message if the task failed"
},
"userFacingErrorMessage": {
"type": "string",
"nullable": true,
"default": null,
"description": "An error message that will be shown to the user"
}
}
}

View File

@@ -260,6 +260,69 @@ class FailingSyncProvider implements IProvider, ISynchronousProvider {
}
}
class FailingSyncProviderWithUserFacingError implements IProvider, ISynchronousProvider {
public const ERROR_MESSAGE = 'Failure';
public const USER_FACING_ERROR_MESSAGE = 'User-facing Failure';
public function getId(): string {
return 'test:sync:fail:user-facing';
}
public function getName(): string {
return self::class;
}
public function getTaskTypeId(): string {
return TextToText::ID;
}
public function getExpectedRuntime(): int {
return 10;
}
public function getOptionalInputShape(): array {
return [
'optionalKey' => new ShapeDescriptor('optional Key', 'AN optional key', EShapeType::Text),
];
}
public function getOptionalOutputShape(): array {
return [
'optionalKey' => new ShapeDescriptor('optional Key', 'AN optional key', EShapeType::Text),
];
}
public function process(?string $userId, array $input, callable $reportProgress): array {
$e = new ProcessingException(self::ERROR_MESSAGE);
$e->setUserFacingMessage(self::USER_FACING_ERROR_MESSAGE);
throw $e;
}
public function getInputShapeEnumValues(): array {
return [];
}
public function getInputShapeDefaults(): array {
return [];
}
public function getOptionalInputShapeEnumValues(): array {
return [];
}
public function getOptionalInputShapeDefaults(): array {
return [];
}
public function getOutputShapeEnumValues(): array {
return [];
}
public function getOptionalOutputShapeEnumValues(): array {
return [];
}
}
class BrokenSyncProvider implements IProvider, ISynchronousProvider {
public function getId(): string {
return 'test:sync:broken-output';
@@ -547,6 +610,7 @@ class TaskProcessingTest extends \Test\TestCase {
$this->providers = [
SuccessfulSyncProvider::class => new SuccessfulSyncProvider(),
FailingSyncProvider::class => new FailingSyncProvider(),
FailingSyncProviderWithUserFacingError::class => new FailingSyncProviderWithUserFacingError(),
BrokenSyncProvider::class => new BrokenSyncProvider(),
AsyncProvider::class => new AsyncProvider(),
AudioToImage::class => new AudioToImage(),
@@ -726,6 +790,36 @@ class TaskProcessingTest extends \Test\TestCase {
self::assertEquals(FailingSyncProvider::ERROR_MESSAGE, $task->getErrorMessage());
}
public function testProviderShouldBeRegisteredAndFailWithUserFacingMessage(): void {
$this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([
new ServiceRegistration('test', FailingSyncProviderWithUserFacingError::class)
]);
self::assertCount(1, $this->manager->getAvailableTaskTypes());
self::assertCount(1, $this->manager->getAvailableTaskTypeIds());
self::assertTrue($this->manager->hasProviders());
$task = new Task(TextToText::ID, ['input' => 'Hello'], 'test', null);
self::assertNull($task->getId());
self::assertEquals(Task::STATUS_UNKNOWN, $task->getStatus());
$this->manager->scheduleTask($task);
self::assertNotNull($task->getId());
self::assertEquals(Task::STATUS_SCHEDULED, $task->getStatus());
$this->eventDispatcher->expects($this->once())->method('dispatchTyped')->with(new IsInstanceOf(TaskFailedEvent::class));
$backgroundJob = new SynchronousBackgroundJob(
Server::get(ITimeFactory::class),
$this->manager,
$this->jobList,
Server::get(LoggerInterface::class),
);
$backgroundJob->start($this->jobList);
$task = $this->manager->getTask($task->getId());
self::assertEquals(Task::STATUS_FAILED, $task->getStatus());
self::assertEquals(FailingSyncProviderWithUserFacingError::ERROR_MESSAGE, $task->getErrorMessage());
self::assertEquals(FailingSyncProviderWithUserFacingError::USER_FACING_ERROR_MESSAGE, $task->getUserFacingErrorMessage());
}
public function testProviderShouldBeRegisteredAndFailOutputValidation(): void {
$this->registrationContext->expects($this->any())->method('getTaskProcessingProviders')->willReturn([
new ServiceRegistration('test', BrokenSyncProvider::class)