Duplicate errors should fail the migration

[hmacr]

Task: https://linear.app/appwrite/issue/SER-417/debug-why-functions-migration-failed-and-caused-user-to-downgrade

Summary by CodeRabbit

• Bug Fixes
  • Standardized error handling during Appwrite migrations: all import failures, including conflict responses, are now treated as errors rather than being skipped.
  • Enhanced logging with clearer resource context to aid troubleshooting and audit trails.
  • Affected items remain marked as errors for the current run, preventing partial successes from masking issues.
  • Users may see more surfaced errors during imports when conflicts occur; successful imports are unaffected.

[coderabbitai]

Walkthrough

The import() method’s catch block in src/Migration/Destinations/Appwrite.php was changed to remove the HTTP 409 special case. All exceptions now set the resource to STATUS_ERROR and log an Exception with resource details, without differentiating conflict errors from other error statuses.

Changes

Cohort / File(s)	Summary
Error handling unification src/Migration/Destinations/Appwrite.php	Removed conditional branch for HTTP 409 in import() error handling. All caught exceptions now set STATUS_ERROR and log an Exception with resource details, eliminating the previous skip-on-409 behavior.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant M as Migrator
    participant D as AppwriteDestination
    participant A as Appwrite API
    participant L as Logger

    rect rgba(230,245,255,0.6)
    note over D: Old flow (before change)
    M->>D: import(resource)
    D->>A: create/update request
    A-->>D: Error (HTTP 409)
    alt HTTP 409 (conflict)
        D->>L: log info/skip (no error)
        note right of D: Resource marked as skipped (not error)
    else Other error
        D->>L: log Exception with details
        note right of D: Resource set to STATUS_ERROR
    end
    end

    rect rgba(235,255,235,0.6)
    note over D: New flow (after change)
    M->>D: import(resource)
    D->>A: create/update request
    A-->>D: Any error (incl. 409)
    D->>L: log Exception with details
    note right of D: Resource set to STATUS_ERROR (uniform)
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through logs and trails of code,
Where 409s once eased the load.
Now every stumble flags the way,
One error path—no shades of gray.
Thump-thump, I note each snag I find,
So future hops run swift, aligned. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly and succinctly captures the core change of treating duplicate (HTTP 409) errors as failures in the migration process, directly reflecting the removal of the previous special-case handling and matching the main intent of the patch.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ser-417

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42ff497 and 8205ef3.

📒 Files selected for processing (1)

• src/Migration/Destinations/Appwrite.php (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/Migration/Destinations/Appwrite.php (3)

src/Migration/Resource.php (6)

• setStatus (168-174)
• Resource (5-236)
• getMessage (176-179)
• getName (108-108)
• getGroup (110-110)
• getId (127-130)

src/Migration/Target.php (2)

• addError (199-202)
• getName (38-38)

src/Migration/Exception.php (1)

• Exception (5-65)

🔇 Additional comments (1)

src/Migration/Destinations/Appwrite.php (1)

241-250: Consistent failure handling looks good.

Marking every caught exception as STATUS_ERROR and surfacing the migration Exception achieves the PR goal of failing on duplicates without introducing regressions. 👍

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}