Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Command/MeilisearchImportCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int

$output->writeln('<info>Importing for index '.$entityClassName.'</info>');

if ($updateSettings) {
$this->settingsUpdater->update($index['prefixed_name'], $responseTimeout, $prefix ? $prefix.$index['prefixed_name'] : null);
}

Comment on lines +117 to +120
Copy link

@coderabbitai coderabbitai bot Aug 5, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Catch & surface SettingsUpdater::update() failures to avoid silent aborts

$this->settingsUpdater->update() is now executed before any data is imported.
If this call throws (e.g. TimeOutException, connectivity issues), the whole import command terminates and no documents are indexed, potentially leaving the index empty.

Guard the call so the operator at least gets a clear-to-read error and, optionally, can continue with document import:

-if ($updateSettings) {
-    $this->settingsUpdater->update($index['prefixed_name'], $responseTimeout, $prefix ? $prefix.$index['prefixed_name'] : null);
-}
+if ($updateSettings) {
+    try {
+        $this->settingsUpdater->update(
+            $index['prefixed_name'],
+            $responseTimeout,
+            $prefix ? $prefix.$index['prefixed_name'] : null
+        );
+    } catch (\Throwable $e) {
+        $output->writeln(
+            sprintf('<error>Failed updating settings for %s: %s</error>', $index['prefixed_name'], $e->getMessage())
+        );
+        // Decide whether to `return self::FAILURE;` or continue importing documents
+        throw $e;
+    }
+}

This mirrors the explicit error handling already present inside formatIndexingResponse().

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ($updateSettings) {
$this->settingsUpdater->update($index['prefixed_name'], $responseTimeout, $prefix ? $prefix.$index['prefixed_name'] : null);
}
if ($updateSettings) {
try {
$this->settingsUpdater->update(
$index['prefixed_name'],
$responseTimeout,
$prefix ? $prefix.$index['prefixed_name'] : null
);
} catch (\Throwable $e) {
$output->writeln(
sprintf(
'<error>Failed updating settings for %s: %s</error>',
$index['prefixed_name'],
$e->getMessage()
)
);
// Decide whether to return self::FAILURE; or continue importing documents
throw $e;
}
}
🤖 Prompt for AI Agents
In src/Command/MeilisearchImportCommand.php around lines 117 to 120, the call to
$this->settingsUpdater->update() can throw exceptions that cause the entire
import command to terminate silently without indexing any documents. Wrap this
call in a try-catch block to catch exceptions like TimeOutException or
connectivity issues, then log or display a clear error message to the operator.
Optionally, allow the import process to continue despite the failure, similar to
the error handling in formatIndexingResponse().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is for a next PR deciding if partial import, when some indexes in the middle fails is okay 😛

Copy link

Choose a reason for hiding this comment

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

@norkunas Fair point! You're right that this PR is focused on the core timeout fix, and the error handling strategy for partial imports is indeed a separate concern that deserves its own focused discussion.

Would you like me to create a follow-up issue to track the error handling improvement for future consideration?


✏️ Learnings added
Learnt from: norkunas
PR: meilisearch/meilisearch-symfony#391
File: src/Command/MeilisearchImportCommand.php:117-120
Timestamp: 2025-08-05T04:46:01.057Z
Learning: In meilisearch-symfony project, the maintainer norkunas prefers to keep PRs focused and defer error handling improvements for settings updates to separate PRs, particularly when it involves decisions about partial import behavior when some indexes fail.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

I'll create a follow-up issue to track the error handling improvement for settings updates.

@norkunas I've created a follow-up issue to track the error handling improvements for settings updates. The issue captures the context from our discussion and outlines the key considerations around partial import behavior when settings updates fail.


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for error handling improvements in MeilisearchImportCommand
gh issue create \
  --title "Improve error handling for settings updates in MeilisearchImportCommand" \
  --body "## Problem

After PR #391, settings updates now occur before document import in the MeilisearchImportCommand. If \`SettingsUpdater::update()\` throws an exception (e.g., TimeOutException, connectivity issues), the entire import command terminates silently without indexing any documents, potentially leaving the index empty.

## Proposed Solution

Add error handling around the settings update call to:
- Catch exceptions and provide clear error messages to operators
- Decide whether to fail fast or continue with document import when settings updates fail
- Consider partial import behavior when some indexes fail in the middle of processing

## Context

This issue was identified during review of PR #391 where the settings update timing was changed to resolve timeout issues.

**Original PR:** https://github.com/meilisearch/meilisearch-symfony/pull/391
**Discussion:** https://github.com/meilisearch/meilisearch-symfony/pull/391#discussion_r2253088004

The error handling strategy for partial imports deserves focused discussion separate from the core timeout fix implemented in PR #391." \
  --assignee norkunas

Length of output: 1281


@norkunas Perfect! I've successfully created issue #392 to track the error handling improvements for settings updates in the MeilisearchImportCommand.

The issue includes:

  • Clear explanation of the problem (settings failures causing silent import aborts)
  • Proposed solutions for error handling and partial import considerations
  • Context linking back to this PR and our discussion
  • Assignment to you for future prioritization

This keeps the current PR focused on the core timeout fix while ensuring the error handling improvements don't get lost. Thanks for keeping the scope clean! 🎯


🧬 Code Graph Analysis Results

Relevant snippets for understanding src/Command/MeilisearchImportCommand.php


File: src/Model/Aggregator.php (lines 12-80)

abstract class Aggregator implements NormalizableInterface
{
    protected string $objectID;
    protected $entity;

    /**
     * @param object $entity
     */
    public function __construct($entity, array $entityIdentifierValues)
    {
        $this->entity = $entity;

        if (\count($entityIdentifierValues) > 1) {
            throw new InvalidEntityForAggregator("Aggregators don't support more than one primary key.");
        }

        $this->objectID = (string) reset($entityIdentifierValues);
    }

    /**
     * Returns the entities class names that should be aggregated.
     *
     * @return string[]
     */
    public static function getEntities(): array
    {
        return [];
    }

    /**
     * Returns an entity id from the provided object id.
     */
    public static function getEntityIdFromObjectId(string $objectId): string
    {
        return $objectId;
    }

    /**
     * Returns an entity class name from the provided object id.
     *
     * @throws EntityNotFoundInObjectID
     */
    public static function getEntityClassFromObjectID(string $objectId): string
    {
        $type = explode('::', $objectId)[0];

        if (\in_array($type, static::getEntities(), true)) {
            return $type;
        }

        throw new EntityNotFoundInObjectID("Entity class from ObjectID $objectId not found.");
    }

    public function normalize(NormalizerInterface $normalizer, ?string $format = null, array $context = []): array
    {
        return array_merge(['objectID' => $this->objectID], $normalizer->normalize($this->entity, $format, $context));
    }
}
  • Abstract class for aggregators used in indexing.
  • Has static method getEntities() returning array of entity class names aggregated.
  • Used in MeilisearchImportCommand::entitiesToIndex() to expand aggregator entities into their constituent entities.

File: src/Services/SettingsUpdater.php (lines 17-79)

final class SettingsUpdater
{
    private const DEFAULT_RESPONSE_TIMEOUT = 5000;

    private Client $searchClient;
    private EventDispatcherInterface $eventDispatcher;
    private Collection $configuration;

    public function __construct(SearchService $searchService, Client $searchClient, EventDispatcherInterface $eventDispatcher)
    {
        $this->searchClient = $searchClient;
        $this->eventDispatcher = $eventDispatcher;
        $this->configuration = $searchService->getConfiguration();
    }

    /**
     * @param non-empty-string  $indice
     * @param positive-int|null $responseTimeout
     */
    public function update(string $indice, ?int $responseTimeout = null, ?string $prefixedName = null): void
    {
        $index = (new Collection($this->configuration->get('indices')))->firstWhere('prefixed_name', $indice);

        if (!\is_array($index)) {
            throw new InvalidIndiceException($indice);
        }

        if (!\is_array($index['settings'] ?? null) || [] === $index['settings']) {
            return;
        }

        $indexName = $prefixedName ?? $index['prefixed_name'];
        $indexInstance = $this->searchClient->index($indexName);
        $responseTimeout = $responseTimeout ?? self::DEFAULT_RESPONSE_TIMEOUT;

        foreach ($index['settings'] as $variable => $value) {
            $method = \sprintf('update%s', ucfirst($variable));

            if (!method_exists($indexInstance, $method)) {
                throw new InvalidSettingName(\sprintf('Invalid setting name: "%s"', $variable));
            }

            if (isset($value['_service']) && $value['_service'] instanceof SettingsProvider) {
                $value = $value['_service']();
            } elseif (('distinctAttribute' === $variable || 'proximityPrecision' === $variable || 'searchCutoffMs' === $variable) && \is_array($value)) {
                $value = $value[0] ?? null;
            }

            // Update
            $task = $indexInstance->{$method}($value);

            // Get task information using uid
            $indexInstance->waitForTask($task['taskUid'], $responseTimeout);
            $task = $indexInstance->getTask($task['taskUid']);

            if ('failed' === $task['status']) {
                throw new TaskException($task['error']['message']);
            }

            $this->eventDispatcher->dispatch(new SettingsUpdatedEvent($index['class'], $indexName, $variable));
        }
    }
}
  • Service used in MeilisearchImportCommand to update index settings.
  • Throws exceptions on invalid index or setting.
  • Waits for Meilisearch task completion and dispatches events on updates.

File: src/Command/IndexCommand.php (lines 13-60)

abstract class IndexCommand extends Command
{
    protected const DEFAULT_RESPONSE_TIMEOUT = 5000;

    protected SearchService $searchService;

    private string $prefix;

    public function __construct(SearchService $searchService)
    {
        $this->searchService = $searchService;
        $this->prefix = $this->searchService->getConfiguration()->get('prefix');

        parent::__construct();
    }

    protected function getEntitiesFromArgs(InputInterface $input, OutputInterface $output): Collection
    {
        $indices = new Collection($this->searchService->getConfiguration()->get('indices'));
        $indexNames = new Collection();

        if ($indexList = $input->getOption('indices')) {
            $list = explode(',', $indexList);
            $indexNames = (new Collection($list))->transform(function (string $item): string {
                // Check if the given index name already contains the prefix
                if (!str_contains($item, $this->prefix)) {
                    return $this->prefix.$item;
                }

                return $item;
            });
        }

        if (0 === \count($indexNames) && 0 === \count($indices)) {
            $output->writeln(
                '<comment>No indices specified. Please either specify indices using the cli option or YAML configuration.</comment>'
            );

            return new Collection();
        }

        if (\count($indexNames) > 0) {
            return $indices->reject(fn (array $item) => !\in_array($item['prefixed_name'], $indexNames->all(), true));
        }

        return $indices;
    }
}
  • Base command class for Meilisearch commands.
  • Provides method getEntitiesFromArgs() to get configured indices filtered by CLI option.
  • Uses prefix from configuration to normalize index names.

File: src/SearchService.php (selected method signatures)

public function getConfiguration(): Collection;

public function isSearchable($className): bool;

public function index(ObjectManager $objectManager, $searchable): array;

public function deleteByIndexName(string $indexName): ?array;
  • getConfiguration() returns configuration collection including indices.
  • isSearchable() checks if a class is searchable.
  • index() indexes entities and returns an array of indexing responses.
  • deleteByIndexName() deletes an index by name.

File: src/Collection.php (selected methods)

  • Implements array-like collection with methods:
    • all(): returns all items as array.
    • forget($keys): removes items by keys.
    • reject($callback): filters out items matching callback.
    • transform($callback): transforms items by callback.
    • firstWhere($key, $operator = null, $value = null): returns first item matching key-value.
    • unique($key = null, bool $strict = false): returns unique items.
  • Used extensively in MeilisearchImportCommand for index collections.

File: src/EventListener/ConsoleOutputSubscriber.php (lines 11-31)

final class ConsoleOutputSubscriber implements EventSubscriberInterface
{
    private OutputStyle $io;

    public function __construct(OutputStyle $io)
    {
        $this->io = $io;
    }

    public function afterSettingsUpdate(SettingsUpdatedEvent $event): void
    {
        $this->io->writeln('<info>Setting "'.$event->getSetting().'" updated of "'.$event->getIndex().'".</info>');
    }

    public static function getSubscribedEvents(): array
    {
        return [
            SettingsUpdatedEvent::class => 'afterSettingsUpdate',
        ];
    }
}
  • Event subscriber that outputs messages to console when settings are updated.
  • Used in MeilisearchImportCommand to provide console feedback.

File: src/Exception/TaskException.php (lines 7-9)

final class TaskException extends \LogicException
{
}
  • Exception thrown when a Meilisearch task fails.
  • Used in MeilisearchImportCommand::formatIndexingResponse().

File: src/Engine.php (index method, lines 28-58)

public function index($searchableEntities): array
{
    if ($searchableEntities instanceof SearchableEntity) {
        $searchableEntities = [$searchableEntities];
    }

    $data = [];
    foreach ($searchableEntities as $entity) {
        $searchableArray = $entity->getSearchableArray();
        if ([] === $searchableArray) {
            continue;
        }

        $indexUid = $entity->getIndexUid();

        if (!isset($data[$indexUid])) {
            $data[$indexUid] = [];
        }

        $data[$indexUid][] = $searchableArray + ['objectID' => $this->normalizeId($entity->getId())];
    }

    $result = [];
    foreach ($data as $indexUid => $objects) {
        $result[$indexUid] = $this->client
            ->index($indexUid)
            ->addDocuments($objects, 'objectID');
    }

    return $result;
}
  • Indexes entities by grouping them per index and adding documents to Meilisearch.
  • Returns array of API responses keyed by index name.

File: src/Command/MeilisearchUpdateSettingsCommand.php (lines 19-93)

  • Command class that updates settings for indices.
  • Has similar entitiesToIndex() method as MeilisearchImportCommand to expand aggregators.
  • Uses SettingsUpdater service to update settings.
  • Adds ConsoleOutputSubscriber for console feedback.

File: src/Command/MeilisearchCreateCommand.php (lines 20-113)

  • Command class that creates indices.
  • Similar pattern of expanding aggregators in entitiesToIndex().
  • Uses SettingsUpdater to update settings optionally.
  • Adds ConsoleOutputSubscriber for console feedback.

File: src/Command/MeilisearchDeleteCommand.php (lines 14-50)

  • Command class that deletes indices.
  • Uses SearchService::deleteByIndexName() to delete indices.
  • Provides console output for success or failure.

File: src/Command/MeilisearchClearCommand.php (lines 12-47)

  • Command class that clears documents from indices.
  • Uses SearchService::clear() method.
  • Provides console output for success or failure.

Summary

The MeilisearchImportCommand class is a Symfony console command that imports entities into Meilisearch indices. It uses:

  • SearchService to check if entities are searchable and to perform indexing.
  • SettingsUpdater to update index settings optionally.
  • ConsoleOutputSubscriber to provide console feedback on events.
  • Collection class to manage index collections and filtering.
  • Aggregator abstract class to expand aggregator entities into their constituent entities.
  • Engine class to perform the actual indexing calls to Meilisearch.
  • Exception classes like TaskException to handle errors during indexing.

The command supports options for batch size, skipping batches, response timeout, updating settings, and swapping indices to avoid downtime. It handles batching of entities, waits for indexing tasks to complete, and can swap temporary indices with live ones.

These related classes and methods provide context for how the import command operates, manages entities, updates settings, and interacts with Meilisearch.


🧠 Learnings used
Learnt from: norkunas
PR: meilisearch/meilisearch-symfony#391
File: src/Command/MeilisearchImportCommand.php:117-120
Timestamp: 2025-08-05T04:46:01.057Z
Learning: In meilisearch-symfony project, the maintainer norkunas prefers to keep PRs focused and defer error handling improvements for settings updates to separate PRs, particularly when it involves decisions about partial import behavior when some indexes fail.

$page = max(0, (int) $input->getOption('skip-batches'));

if ($page > 0) {
Expand Down Expand Up @@ -155,10 +159,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
} while (\count($entities) >= $batchSize);

$manager->clear();

if ($updateSettings) {
$this->settingsUpdater->update($index['prefixed_name'], $responseTimeout, $prefix ? $prefix.$index['prefixed_name'] : null);
}
}

if ($swapIndices) {
Expand Down
28 changes: 15 additions & 13 deletions tests/Integration/Command/MeilisearchImportCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,17 @@ public function testSearchImportAggregator(): void
$commandTester = new CommandTester($command);
$return = $commandTester->execute(['--indices' => $this->index->getUid()]);

$this->assertSame('Importing for index Meilisearch\Bundle\Tests\Entity\Post
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (6 indexed since start)
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (6 indexed since start)
$this->assertSame(<<<'EOD'
Importing for index Meilisearch\Bundle\Tests\Entity\Post
Setting "stopWords" updated of "sf_phpunit__posts".
Setting "filterableAttributes" updated of "sf_phpunit__posts".
Setting "searchCutoffMs" updated of "sf_phpunit__posts".
Setting "typoTolerance" updated of "sf_phpunit__posts".
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (6 indexed since start)
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (6 indexed since start)
Done!
', $commandTester->getDisplay());

EOD, $commandTester->getDisplay());

$this->assertSame(0, $return);
}
Expand Down Expand Up @@ -262,12 +264,12 @@ public function testImportingIndexNameWithAndWithoutPrefix(): void

$this->assertSame(<<<'EOD'
Importing for index Meilisearch\Bundle\Tests\Entity\Post
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (6 indexed since start)
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (6 indexed since start)
Setting "stopWords" updated of "sf_phpunit__posts".
Setting "filterableAttributes" updated of "sf_phpunit__posts".
Setting "searchCutoffMs" updated of "sf_phpunit__posts".
Setting "typoTolerance" updated of "sf_phpunit__posts".
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (6 indexed since start)
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (6 indexed since start)
Done!

EOD, $commandTester->getDisplay());
Expand All @@ -291,12 +293,12 @@ public function testImportingIndexNameWithAndWithoutPrefix(): void
$output = $commandTester->getDisplay();
$this->assertSame(<<<'EOD'
Importing for index Meilisearch\Bundle\Tests\Entity\Post
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (6 indexed since start)
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (6 indexed since start)
Setting "stopWords" updated of "sf_phpunit__posts".
Setting "filterableAttributes" updated of "sf_phpunit__posts".
Setting "searchCutoffMs" updated of "sf_phpunit__posts".
Setting "typoTolerance" updated of "sf_phpunit__posts".
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (6 indexed since start)
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (6 indexed since start)
Done!

EOD, $commandTester->getDisplay());
Expand Down Expand Up @@ -393,11 +395,11 @@ public function testImportWithDynamicSettings(): void

$this->assertSame(<<<'EOD'
Importing for index Meilisearch\Bundle\Tests\Entity\DynamicSettings
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\DynamicSettings entities into sf_phpunit__dynamic_settings index (6 indexed since start)
Setting "filterableAttributes" updated of "sf_phpunit__dynamic_settings".
Setting "searchableAttributes" updated of "sf_phpunit__dynamic_settings".
Setting "stopWords" updated of "sf_phpunit__dynamic_settings".
Setting "synonyms" updated of "sf_phpunit__dynamic_settings".
Indexed a batch of 6 / 6 Meilisearch\Bundle\Tests\Entity\DynamicSettings entities into sf_phpunit__dynamic_settings index (6 indexed since start)
Done!

EOD, $importOutput);
Expand Down Expand Up @@ -432,14 +434,14 @@ public function testImportUpdatesSettingsOnce(): void

$this->assertSame(<<<'EOD'
Importing for index Meilisearch\Bundle\Tests\Entity\Post
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (2 indexed since start)
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (2 indexed since start)
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (4 indexed since start)
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (4 indexed since start)
Setting "stopWords" updated of "sf_phpunit__posts".
Setting "filterableAttributes" updated of "sf_phpunit__posts".
Setting "searchCutoffMs" updated of "sf_phpunit__posts".
Setting "typoTolerance" updated of "sf_phpunit__posts".
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (2 indexed since start)
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (2 indexed since start)
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__posts index (4 indexed since start)
Indexed a batch of 2 / 2 Meilisearch\Bundle\Tests\Entity\Post entities into sf_phpunit__aggregated index (4 indexed since start)
Done!

EOD, $importOutput);
Expand Down
Loading