fix: return 409 when a push target update reuses an existing identifier by ChiragAgg5k · Pull Request #13789 · appwrite/appwrite · GitHub
Skip to content

fix: return 409 when a push target update reuses an existing identifier - #13789

Merged
ChiragAgg5k merged 1 commit into
mainfrom
fix-push-target-update-duplicate
Sep 21, 2026
Merged

ChiragAgg5k merged 1 commit into
mainfrom
fix-push-target-update-duplicate

Conversation

@ChiragAgg5k

Copy link
Copy Markdown
Member

What does this PR do?

PUT /v1/account/targets/:targetId/push now returns 409 user_target_already_exists when the new identifier is already registered on another target, instead of an unhandled 500. Fixes Sentry CLOUD-3QX3, 133 events since 9/5.

Test Plan

Extended testUpdatePushTarget in AccountCustomClientTest: creates a second push target, updates the first to the second's identifier, asserts 409 with the user_target_already_exists type, then reads the account and asserts the first target's identifier is unchanged. Verified red (500) with the controller hunk reverted and green with it applied.

Related PRs and Issues

The create push target endpoint already maps Duplicate to USER_TARGET_ALREADY_EXISTS; this brings the update path in line.

Checklist

Updating a push target to an identifier already registered on another target hit the unique index on targets and surfaced as an unhandled 500 (Sentry CLOUD-3QX3). Create push target already maps this to USER_TARGET_ALREADY_EXISTS; the update path now does the same.
@greptile-apps

greptile-apps Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown

✨ Benchmark results

Comparing main (before) → fix-push-target-update-duplicate (after).

Metric Before After Change
🚀 Requests/sec 155.68 156.2 ⚪ +0.3%
⏱️ Latency P50 110.4 ms 111.18 ms ⚪ +0.7%
⏱️ Latency P95 258.92 ms 256.44 ms ⚪ -1%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 111.18 256.44 10,146 156.2 -2.48
Account 211.62 390.93 534 8.86 -31.36
TablesDB 107.67 210.61 5,518 87.68 -0.13
Storage 102.14 220.68 2,670 43.92 +0.3
Functions 150.12 307.46 1,424 24.04 +1.54

Top API waits (after)

API request Max wait (ms)
tablesdb.rows.increment 647.64
account.name.update 574.19
account.prefs.update 561.69
functions.variables.update 547
tablesdb.rows.delete 532.73

@ChiragAgg5k
ChiragAgg5k merged commit df6b00f into main Sep 21, 2026
50 checks passed
@ChiragAgg5k
ChiragAgg5k deleted the fix-push-target-update-duplicate branch September 21, 2026 10:10
cursor Bot pushed a commit that referenced this pull request Sep 21, 2026
Take main's push-target duplicate E2E coverage from #13789 and keep
the concurrent phone-token helper from this branch.

Co-authored-by: Jake Barnby <abnegate@users.noreply.github.com>
abnegate added a commit that referenced this pull request Sep 22, 2026
Drop the account.php and AccountCustomClientTest changes: the phone token
and push target duplicate handling landed on main via #13789 and #13790
with 409 semantics. Keep only the teams membership Duplicate mapping and
its concurrent E2E coverage.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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