Skip to content

chore: guard metadata collection service to not fail the context_chat outputs - #266

Merged
kyteinsky merged 5 commits into
mainfrom
chore/guard-metadata-service
Oct 15, 2026
Merged

kyteinsky merged 5 commits into
mainfrom
chore/guard-metadata-service

Conversation

@kyteinsky

Copy link
Copy Markdown
Contributor

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@kyteinsky
kyteinsky requested a review from lukasdotcom August 11, 2026 12:32
@kyteinsky
kyteinsky force-pushed the chore/guard-metadata-service branch 2 times, most recently from 07549bb to f30f7d6 Compare August 13, 2026 14:13
… outputs

Signed-off-by: kyteinsky <kyteinsky@gmail.com>
Signed-off-by: kyteinsky <kyteinsky@gmail.com>
@kyteinsky
kyteinsky force-pushed the chore/guard-metadata-service branch from f30f7d6 to 8a79716 Compare September 30, 2026 09:30
continue;
} catch (InvalidArgumentException $e) {
$this->logger->warning('Could not extract item id from the source id', ['source_id' => $source['source_id'], 'exception' => $e]);
continue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would perhaps be even better to simply catch \Throwable here, as loading the provider class can fail in unexpected ways (who knows what people do in their apps)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe, fair enough, that is already implemented in the lines below where $url = $klass->getItemUrl($itemId); their function is called. In this try block, it's only our and server code so should be good.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Server::get($providerConfig['classString']) loads third party php files, no? If those throw an error that is not ContainerException, NotFoundExceptionInterface or InvalidArgumentException, then this whole method throws.

@kyteinsky
kyteinsky merged commit 7af50cd into main Oct 15, 2026
56 of 59 checks passed
@kyteinsky
kyteinsky deleted the chore/guard-metadata-service branch October 15, 2026 12:09
@kyteinsky kyteinsky mentioned this pull request Oct 15, 2026
1 task
continue;
} catch (InvalidArgumentException $e) {
$this->logger->warning('Could not extract item id from the source id', ['source_id' => $source['source_id'], 'exception' => $e]);
continue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Server::get($providerConfig['classString']) loads third party php files, no? If those throw an error that is not ContainerException, NotFoundExceptionInterface or InvalidArgumentException, then this whole method throws.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants