fix(settings): report a failed settings write instead of reporting success

[waleedlatif1]

The route cannot fail

app/api/users/me/settings/route.ts:75

} catch (error: any) {
  logger.error(`[${requestId}] Settings update error`, error)
  return NextResponse.json({ success: true }, { status: 200 })
}

A failed upsert is answered exactly like a successful one.

Which makes the client's rollback unreachable

useUpdateGeneralSetting (hooks/queries/general-settings.ts:166) is a full optimistic mutation:

• onMutate writes the new value into the cache and calls syncThemeToNextThemes
• onError restores context.previousSettings and re-syncs the theme

requestJson throws ApiClientError only on a non-2xx. Since the route always answered 200, onError could never run — the rollback and its theme re-sync are dead code.

What the user sees

The upsert throws (pool exhaustion, a conflict on settings.userId, a replica failover). The UI applies the change. Nothing is persisted. onSettled invalidates, the refetch returns the old value, and the setting quietly reverts with no error anywhere.

For a consent-shaped setting — telemetry, email opt-out — the user believes they opted out and did not.

The fix

The catch returns 500, which is what the mutation was already written to handle. No client change needed.

Deliberately not changed

GET still falls back to defaultUserSettings on error, also at 200. Failing it would take the settings page down on a transient read, and whether that trade is right is a separate judgement from this one. Worth noting it compounds the above — a failing GET makes the post-write refetch show defaults — but the write path is the one making a false claim.

Testing

A route test drives the failure through the real handler rather than asserting on a mock:

✓ reports success when the write lands
✓ reports failure when the write throws

Verified it can fail — restoring the 200 turns the second case red with expected 200 to be 500.

bun run type-check clean; 41 tests passing across app/api/users and the settings hook.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 24, 2026 1:56am

[cursor]

PR Summary

Medium Risk
Touches the user-settings write path (including consent-like fields) and changes the error contract from always-200 to 500. Scope is a one-line catch-handler fix plus a regression test.

Overview
Fixes PATCH /api/users/me/settings so a failed upsert no longer returns { success: true } with 200.

The catch now returns 500 with an error body. That lets the existing optimistic useUpdateGeneralSetting mutation run onError and roll back the cache (including consent-shaped settings) instead of showing a write that never persisted.

Adds a route test that asserts success on a real write and failure when the insert throws. GET still returns defaults on error; that path is unchanged.

^{Reviewed by Cursor Bugbot for commit c4b6c13. Configure here.}

[greptile-apps]

Greptile Summary

The PR corrects the settings PATCH endpoint so failed database writes return HTTP 500, allowing existing optimistic client mutations to execute their rollback behavior.

• Replaces the false HTTP 200 success response in the write-error path with a structured 500 error.
• Adds route-level regression coverage for successful and failed settings writes.

Confidence Score: 5/5

The PR appears safe to merge because failed settings writes now reach the client’s existing error and rollback path without introducing a conflicting response contract.

The route wrapper preserves the returned 500 response, requestJson rejects it, and the optimistic settings mutation restores its previous cached state through onError; the added tests exercise the actual mocked database write path.

Important Files Changed

Filename	Overview
apps/sim/app/api/users/me/settings/route.ts	Correctly returns a non-success status when a settings upsert throws, matching the existing client error and rollback contract.
apps/sim/app/api/users/me/settings/route.test.ts	Adds correctly wired regression tests proving successful writes return 200 and thrown database writes return 500.

_{Reviews (1): Last reviewed commit: "fix(settings): report a failed settings ..." | Re-trigger Greptile}