8000 Fix: Duplicate document error while creating file by lohanidamodar · Pull Request #10891 · appwrite/appwrite · GitHub
[go: up one dir, main page]
Skip to content

Fix: Duplicate document error while creating file #10891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
abnegate merged 2 commits into from
Dec 3, 2025
Merged

Fix: Duplicate document error while creating file #10891

abnegate merged 2 commits into from
Dec 3, 2025
+9 −1

Conversation

@lohanidamodar
Copy link
Member
@lohanidamodar lohanidamodar commented Dec 2, 2025

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Contributor
coderabbitai bot commented Dec 2, 2025 •
edited
Loading
📝 Walkthrough

Walkthrough

The pull request adds error handling to the storage controller's document creation logic. When creating file documents in bucket-specific collections, the code now wraps createDocument calls in try/catch blocks to intercept and convert specific exceptions into standardized storage errors: DuplicateException becomes STORAGE_FILE_ALREADY_EXISTS, and NotFoundException becomes STORAGE_BUCKET_NOT_FOUND. This pattern is consistently applied across multiple code paths for file creation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single file modified with repetitive, consistent error handling pattern
  • Exception-to-error mapping logic is straightforward and predictable
  • Potential areas requiring attention during review:
    • Verify all exception types (DuplicateException, NotFoundException) are correctly identified and mapped
    • Confirm error code constants (STORAGE_FILE_ALREADY_EXISTS, STORAGE_BUCKET_NOT_FOUND) are properly defined elsewhere in the codebase
    • Ensure no additional exception scenarios should be caught and handled in these code paths
    • Check that the try/catch scope is appropriately narrow to avoid masking unintended exceptions

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is a contribution template with no substantive content provided; sections are empty or contain only placeholders, making it impossible to verify relevance to the changeset. Provide actual details about what the PR does, why it's needed, and how changes were tested to replace the template.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly relates to the main change: adding error handling for duplicate document exceptions when creating files in storage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 fix-duplicate-document-error
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 088359c and d0b1f5f.

📒 Files selected for processing (1)
  • app/controllers/api/storage.php (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings 
Learnt from: ItzNotABug
Repo: appwrite/appwrite PR: 9661
File: app/controllers/api/migrations.php:332-337
Timestamp: 2025-04-17T08:07:54.565Z
Learning: In the Appwrite codebase, error handling intentionally uses the same error codes for different failure conditions (like STORAGE_BUCKET_NOT_FOUND for both non-existent buckets and authorization failures) as a security measure to prevent information disclosure about what specifically went wrong.
📚 Learning: 2025-04-17T08:07:54.565Z 
Learnt from: ItzNotABug
Repo: appwrite/appwrite PR: 9661
File: app/controllers/api/migrations.php:332-337
Timestamp: 2025-04-17T08:07:54.565Z
Learning: In the Appwrite codebase, error handling intentionally uses the same error codes for different failure conditions (like STORAGE_BUCKET_NOT_FOUND for both non-existent buckets and authorization failures) as a security measure to prevent information disclosure about what specifically went wrong.

Applied to files:

  • app/controllers/api/storage.php
🧬 Code graph analysis (1)
app/controllers/api/storage.php (1)
src/Appwrite/Extend/Exception.php (1)
  • Exception (7-465)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: scan
🔇 Additional comments (2)
app/controllers/api/storage.php (2)

684-690: Good addition of race condition handling.

This try/catch properly handles concurrent file creation attempts. Between the file existence check at line 571 and the document creation here, another request could create the same file, resulting in a DuplicateException. Converting it to STORAGE_FILE_ALREADY_EXISTS provides clear, user-friendly feedback.

The NotFoundException handling also covers the edge case where the bucket collection no longer exists, converting it appropriately to STORAGE_BUCKET_NOT_FOUND.

Based on learnings, this aligns with Appwrite's security approach of using consistent error codes.

740-741: Consistent error handling for chunked upload path.

This addition mirrors the error handling at lines 686-687, ensuring both file creation paths (complete and chunked upload placeholders) have consistent race condition protection. The NotFoundException handling at lines 742-743 was already in place, so this completes the error handling for this code path.

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

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

@github-actions
Copy link
github-actions bot commented Dec 2, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
imagemagick 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-c++ 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-dev 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-heic 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-jpeg 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-jxl 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-libs 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-openexr 7.1.2.3-r0 CVE-2025-62171 HIGH
imagemagick-webp 7.1.2.3-r0 CVE-2025-62171 HIGH
libecpg 17.6-r0 CVE-2025-12818 HIGH
libecpg-dev 17.6-r0 CVE-2025-12818 HIGH
libpng 1.6.47-r0 CVE-2025-64720 HIGH
libpng 1.6.47-r0 CVE-2025-65018 HIGH
libpng-dev 1.6.47-r0 CVE-2025-64720 HIGH
libpng-dev 1.6.47-r0 CVE-2025-65018 HIGH
libpq 17.6-r0 CVE-2025-12818 HIGH
libpq-dev 17.6-r0 CVE-2025-12818 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
postgresql17-dev 17.6-r0 CVE-2025-12818 HIGH
github.com/containerd/containerd/v2 v2.0.2 CVE-2024-25621 HIGH
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH
stdlib 1.22.10 CVE-2025-58183 HIGH
stdlib 1.22.10 CVE-2025-58186 HIGH
stdlib 1.22.10 CVE-2025-58187 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@github-actions
Copy link
github-actions bot commented Dec 2, 2025 •
edited
Loading

✨ Benchmark results

  • Requests per second: 1,062
  • Requests with 200 status code: 191,180
  • P99 latency: 0.178514908

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,062 1,087
200 191,180 195,742
P99 0.178514908 0.188156221

@abnegate abnegate merged commit 379b85b into 1.8.x Dec 3, 2025
41 checks passed
@abnegate abnegate deleted the fix-duplicate-document-error branch December 3, 2025 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abnegate abnegate abnegate approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lohanidamodar @abnegate
0