diff --git a/src/Controller/InfoProviderController.php b/src/Controller/InfoProviderController.php index 1a3e65f0d..8cf267376 100644 --- a/src/Controller/InfoProviderController.php +++ b/src/Controller/InfoProviderController.php @@ -218,7 +218,7 @@ public function search(Request $request, #[MapEntity(id: 'target')] ?Part $updat } #[Route('/from_url', name: 'info_providers_from_url')] - public function fromURL(Request $request, CreateFromUrlHelper $fromUrlHelper): Response + public function fromURL(Request $request, CreateFromUrlHelper $fromUrlHelper, LoggerInterface $exceptionLogger): Response { $this->denyAccessUnlessGranted('@info_providers.create_parts'); @@ -271,6 +271,11 @@ public function fromURL(Request $request, CreateFromUrlHelper $fromUrlHelper): R } } catch (ExceptionInterface $e) { $this->addFlash('error', t('info_providers.search.error.general_exception', ['%type%' => (new \ReflectionClass($e))->getShortName()])); + } catch (\RuntimeException $e) { + //Same handling as the search page: a provider which rejects the request (an unknown model or an + //exhausted quota, for example) is a normal outcome here and has to be shown, not turned into a 500 + $this->addFlash('error', t('info_providers.search.error.general_exception', ['%type%' => (new \ReflectionClass($e))->getShortName()])); + $exceptionLogger->error('Error while creating a part from an URL: '.$e->getMessage(), ['exception' => $e]); } } diff --git a/src/Services/InfoProviderSystem/Providers/AIWebProvider.php b/src/Services/InfoProviderSystem/Providers/AIWebProvider.php index feb943847..3e569de38 100644 --- a/src/Services/InfoProviderSystem/Providers/AIWebProvider.php +++ b/src/Services/InfoProviderSystem/Providers/AIWebProvider.php @@ -37,12 +37,14 @@ use League\HTMLToMarkdown\HtmlConverter; use Psr\Cache\CacheItemPoolInterface; use Symfony\AI\Platform\Message\Message; +use Symfony\AI\Platform\Result\DeferredResult; use Symfony\AI\Platform\Message\MessageBag; use Symfony\Component\DomCrawler\Crawler; use Symfony\Component\DomCrawler\UriResolver; use Symfony\Component\HttpClient\NoPrivateNetworkHttpClient; use Symfony\Component\Intl\Languages; use Symfony\Contracts\HttpClient\HttpClientInterface; +use Symfony\Contracts\HttpClient\ResponseInterface; use function Symfony\Component\String\u; @@ -55,6 +57,9 @@ final class AIWebProvider implements InfoProviderInterface private const DISTRIBUTOR_NAME = 'Website'; + /** @var int How much of a failed provider response is quoted in the error message */ + private const MAX_REPORTED_RESPONSE_LENGTH = 500; + private readonly HttpClientInterface $httpClient; public function __construct( @@ -288,11 +293,54 @@ private function callLLM(string $htmlContent, string $url, ?string $structuredDa 'json_schema' => $this->jsonSchemaConverter->getJSONSchema(), ] ]); + //The platform returns a deferred result: the request is only really carried out (and the answer + //converted) when the result is read. Reading it outside this try would let a provider error - a + //rejected model, an exhausted quota, an invalid key - escape as an unhandled exception, which ends + //the whole request with a 500 instead of the error message this catch was written for. + return $result->getResult()->getContent(); } catch (\Throwable $e) { - throw new \RuntimeException('LLM invocation failed: '.$e->getMessage(), previous: $e); + throw new \RuntimeException( + 'LLM invocation failed: '.$e->getMessage().$this->describeProviderResponse($result ?? null), + previous: $e + ); + } + } + + /** + * Describes what the provider actually answered, for the message of a failed invocation. + * + * The exceptions of the platform only carry what its converter made of the answer, and that can be as + * unhelpful as "Provider returned error" - the wording a gateway like OpenRouter uses when the model + * provider behind it refused, with the reason in a field the converter drops. The raw response is still + * around at this point, so the status code and the beginning of the body are taken from there: without + * them, an administrator has nothing to act on. + * + * @return string The description, or an empty string if the response is not available + */ + private function describeProviderResponse(?DeferredResult $result): string + { + if (!$result instanceof DeferredResult) { + return ''; } - return $result->getResult()->getContent(); + try { + $response = $result->getRawResult()->getObject(); + + if (!$response instanceof ResponseInterface) { + return ''; + } + + //false: the body of an error response is wanted here, not another exception + $body = trim($response->getContent(false)); + + return sprintf(' (provider answered HTTP %d: %s)', $response->getStatusCode(), + mb_strlen($body) > self::MAX_REPORTED_RESPONSE_LENGTH + ? mb_substr($body, 0, self::MAX_REPORTED_RESPONSE_LENGTH).'...' + : $body); + } catch (\Throwable) { + //Whatever went wrong while describing the failure must not replace the failure itself + return ''; + } } private function buildSystemPrompt(): string diff --git a/tests/Services/InfoProviderSystem/Providers/AIWebProviderTest.php b/tests/Services/InfoProviderSystem/Providers/AIWebProviderTest.php new file mode 100644 index 000000000..db3c358b5 --- /dev/null +++ b/tests/Services/InfoProviderSystem/Providers/AIWebProviderTest.php @@ -0,0 +1,186 @@ +. + */ + +declare(strict_types=1); + +namespace App\Tests\Services\InfoProviderSystem\Providers; + +use App\Services\AI\AIPlatformRegistry; +use App\Services\AI\AIPlatforms; +use App\Services\InfoProviderSystem\DTOJsonSchemaConverter; +use App\Services\InfoProviderSystem\CreateFromUrlHelper; +use App\Services\InfoProviderSystem\Providers\AIWebProvider; +use App\Services\InfoProviderSystem\SubmittedPageStorage; +use App\Settings\InfoProviderSystem\AIExtractorSettings; +use App\Tests\SettingsTestHelper; +use Jbtronics\SettingsBundle\Manager\SettingsManagerInterface; +use PHPUnit\Framework\TestCase; +use Symfony\AI\Platform\Exception\BadRequestException; +use Symfony\AI\Platform\Model; +use Symfony\AI\Platform\ModelCatalog\ModelCatalogInterface; +use Symfony\AI\Platform\PlatformInterface; +use Symfony\AI\Platform\Result\DeferredResult; +use Symfony\AI\Platform\Result\InMemoryRawResult; +use Symfony\AI\Platform\Result\RawHttpResult; +use Symfony\AI\Platform\ResultConverterInterface; +use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\HttpClient\MockHttpClient; +use Symfony\Component\HttpClient\Response\MockResponse; +use Symfony\Contracts\HttpClient\HttpClientInterface; + +/** + * @see AIWebProvider + */ +final class AIWebProviderTest extends TestCase +{ + /** + * Builds a platform whose result fails when it is read, which is how a provider error really arrives: + * invoke() only hands out a deferred result, and the request is carried out when that result is used. + */ + private function platformFailingOnRead(\Throwable $failure): PlatformInterface + { + $converter = $this->createMock(ResultConverterInterface::class); + $converter->method('supports')->willReturn(true); + $converter->method('convert')->willThrowException($failure); + + $deferred = new DeferredResult($converter, new InMemoryRawResult([], [], (object) [])); + + return new class($deferred) implements PlatformInterface { + public function __construct(private readonly DeferredResult $deferred) + { + } + + public function invoke(string|Model $model, array|string|object $input, array $options = []): DeferredResult + { + return $this->deferred; + } + + public function getModelCatalog(): ModelCatalogInterface + { + throw new \LogicException('Not needed for this test'); + } + }; + } + + + /** + * The same, but with a real HTTP response behind the result, so that the description of the failure can be + * checked - a gateway puts the actual reason into the body, not into the message of the exception. + */ + private function platformFailingWithResponse(\Throwable $failure, int $status, string $body): PlatformInterface + { + $converter = $this->createMock(ResultConverterInterface::class); + $converter->method('supports')->willReturn(true); + $converter->method('convert')->willThrowException($failure); + + $response = (new MockHttpClient(new MockResponse($body, ['http_code' => $status]))) + ->request('POST', 'https://invalid.invalid/chat'); + + $deferred = new DeferredResult($converter, new RawHttpResult($response)); + + return new class($deferred) implements PlatformInterface { + public function __construct(private readonly DeferredResult $deferred) + { + } + + public function invoke(string|Model $model, array|string|object $input, array $options = []): DeferredResult + { + return $this->deferred; + } + + public function getModelCatalog(): ModelCatalogInterface + { + throw new \LogicException('Not needed for this test'); + } + }; + } + + /** + * callLLM() only uses the platform registry, the settings and the schema converter. Building the whole + * provider would drag in three more services which have nothing to do with this, so the instance is created + * without its constructor and only those three are filled in. + */ + private function provider(PlatformInterface $platform): AIWebProvider + { + //The registry is final, so it is built for real: one registered platform, reported as enabled + $settingsManager = $this->createMock(SettingsManagerInterface::class); + $settingsManager->method('get')->willReturn(new class { + public function isAIPlatformEnabled(): bool + { + return true; + } + }); + $registry = new AIPlatformRegistry($settingsManager, [AIPlatforms::OPENROUTER->toServiceTagName() => $platform]); + + $settings = SettingsTestHelper::createSettingsDummy(AIExtractorSettings::class); + $settings->platform = AIPlatforms::OPENROUTER; + $settings->model = 'a/model'; + + $provider = (new \ReflectionClass(AIWebProvider::class))->newInstanceWithoutConstructor(); + + foreach (['AIPlatformRegistry' => $registry, 'settings' => $settings, + 'jsonSchemaConverter' => new DTOJsonSchemaConverter()] as $name => $value) { + $property = new \ReflectionProperty(AIWebProvider::class, $name); + $property->setValue($provider, $value); + } + + return $provider; + } + + public function testAProviderErrorWhileReadingTheResultIsWrapped(): void + { + //A rejected model, an exhausted quota or an invalid key all arrive like this. Before, the result was + //read outside the try block, so the exception escaped unhandled and ended the request with a 500 - + //even though both the search page and the "create from URL" page know how to report a RuntimeException. + $provider = $this->provider($this->platformFailingOnRead(new BadRequestException('Provider returned error'))); + + $callLLM = new \ReflectionMethod(AIWebProvider::class, 'callLLM'); + + try { + $callLLM->invoke($provider, 'a page', 'https://invalid.invalid/part'); + self::fail('Expected the provider error to be reported'); + } catch (\RuntimeException $e) { + self::assertStringContainsString('LLM invocation failed', $e->getMessage()); + self::assertStringContainsString('Provider returned error', $e->getMessage()); + self::assertInstanceOf(BadRequestException::class, $e->getPrevious()); + } + } + + public function testTheAnswerOfTheProviderIsPartOfTheMessage(): void + { + //"Provider returned error" is what a gateway says when the model provider behind it refused; the reason + //is in the body it sent along. Without it an administrator cannot tell an exhausted quota from a + //rejected request, so the status and the body belong in the message. + $body = '{"error":{"message":"Provider returned error","code":400,"metadata":{"raw":"quota exceeded"}}}'; + $provider = $this->provider($this->platformFailingWithResponse( + new BadRequestException('Provider returned error'), 400, $body + )); + + $callLLM = new \ReflectionMethod(AIWebProvider::class, 'callLLM'); + + try { + $callLLM->invoke($provider, 'a page', 'https://invalid.invalid/part'); + self::fail('Expected the provider error to be reported'); + } catch (\RuntimeException $e) { + self::assertStringContainsString('provider answered HTTP 400', $e->getMessage()); + self::assertStringContainsString('quota exceeded', $e->getMessage()); + } + } +}