Make Appwrite source report resilient to per-resource failures#199
Make Appwrite source report resilient to per-resource failures#199premtsd-code wants to merge 8 commits into
Conversation
Probe reachability first, then report each resource group independently: an unsupported resource (404 on an older/partial source) is recorded and skipped instead of aborting the whole report. Invalid credentials (401) and missing scopes (403) are still surfaced.
Greptile SummaryThis PR changes Appwrite source reporting to classify failures before returning resource counts. The main changes are:
Confidence Score: 4/5The report classification changes are focused, but optional endpoint isolation needs attention before this is safe to merge. The affected code paths are localized to Appwrite resource reporting, and the issues are concrete control-flow regressions where optional endpoint failures can abort unrelated discovery. src/Migration/Sources/Appwrite.php
 T-Rex LogsWhat T-Rex did
Reviews (7): Last reviewed commit: "Stop suppressing Appwrite report failure..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Addresses review: only a 404 (resource unsupported by the source) is skippable. Missing scopes (403) — including those previously swallowed by inner catches in integrations/projects/domains — now surface as 'Missing required scopes'. Invalid key, rate limit, server, and connectivity errors fail fast instead of returning a partial report. Removes group-level addError recording that status counters dropped.
Per review: drop the 404 skip. A 404 is overloaded (item-level vs unsupported-service) and silent skips hid groups from getErrors(). report() now fails on any non-403 error, matching its original fail-fast behavior, while still aggregating 403s across groups and letting optional reporters' inner catches propagate 403 instead of zeroing it.
rethrowAuthErrors() now re-throws 401 as well as 403, so an invalid key surfaced only by an optional reporter (platforms/keys/variables/email templates/rules) fails fast instead of returning a zero count.
When an invalid key is first detected by an optional reporter, report() now throws the intended credential error instead of leaking the raw SDK exception.
Arrow functions capture by value, so each reporter mutated a discarded copy of $report and report() returned only the version. Use first-class callables and pass resources/report/resourceIds as arguments at call time.
| } catch (\Throwable) { | ||
| $report[Resource::TYPE_RULE] = 0; | ||
| } | ||
| $report[Resource::TYPE_RULE] = $this->proxy->listRules([Query::limit(1)])->total; |
There was a problem hiding this comment.
This optional reporter no longer catches non-auth endpoint failures. On an older Appwrite source, or when domain rules are disabled, listRules() can return 404 and the outer report loop rethrows it, aborting discovery for every later group. The intended behavior for optional reporters is to keep returning 0 for unsupported resources while still surfacing credential and scope errors.
| } catch (\Throwable) { | ||
| $report[Resource::TYPE_PROJECT_VARIABLE] = 0; | ||
| } | ||
| $report[Resource::TYPE_PROJECT_VARIABLE] = $this->project->listVariables($variableQueries)->total; |
There was a problem hiding this comment.
listVariables() now runs without the optional-resource catch and it executes before the singleton project counts are written. If variables are unavailable on the source and return 404, reportProjects() exits before setting project-protocols, project-labels, project-services, or email templates, even though those counts do not depend on variables. Isolate the optional endpoint failure so unrelated project resources can still be reported.
| } catch (\Throwable) { | ||
| $report[Resource::TYPE_PLATFORM] = 0; | ||
| } | ||
| $report[Resource::TYPE_PLATFORM] = $this->project->listPlatforms($platformQueries)->total; |
There was a problem hiding this comment.
The integration reporter now lets listPlatforms() throw directly. If platforms are unsupported or disabled on the source, this aborts the whole integrations group before API keys, webhooks, and the SMTP singleton are counted. Keep each optional integration endpoint isolated so one unsupported resource returns 0 without hiding the rest of the group.
1 participant
Problem
Appwrite::report()wrapped every resource group in a single try/catch and rethrew on the first failure. A migration source on an older version (or with a service disabled) would 404 on one resource type, aborting discovery of every resource — the wizard then showed a generic "couldn't load resources" with no way to distinguish a real connectivity problem, an invalid key, or a missing scope.Change
/health/version(unauthenticated) before anything else, so a wrong/unreachable endpoint fails with a clear, distinct error.$reportis passed by reference at call time). The per-group catch routes failures by intent:Missing required scopes for: <groups>.0count, but now re-throw credential (401) and scope (403) failures viarethrowAuthErrors()so those are never silently zeroed.Notes
No public API change —
report()still returns the report array and throws on credential/connectivity failures. The behavioural change is that failures are now classified (reachable, invalid key, missing scopes, other) instead of collapsing into one opaque error.