Repository navigation
Retry device registration when a concurrent request wins the race - #135
Merged
Merged
Conversation
Two first-time registrations of the same push token could both pass the uniqueness validation; the unique index then rejected the later INSERT with RecordNotUnique, a 500. Retry once: the lookup finds the row the other request created and updates it (200), like any re-registration. Reassigning a token to the caller stays as is: it's the documented way a device moves to whoever signs in on it, and pushes go to the device, so someone who obtained another person's token only redirects their own notifications to that device. Also update the AGENTS.md test count (505 tests after #134 and this). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L4Pma9VAqB5A22KCEEin9R
The second connection read :user from configuration_hash, but the key is :username, so it only worked where the OS user matches the DB user. CI connects as postgres through DATABASE_URL and failed. Use the test connection's own conninfo, and only skip the callback if it was added. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L4Pma9VAqB5A22KCEEin9R
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two simultaneous first-time
POST /deviceswith the same(platform, token), for example an app that registers twice at launch, can both pass the uniqueness validation. The unique index then rejects the later INSERT withRecordNotUnique, which surfaces as a 500.Fix
DevicesController#save_deviceretries once onRecordNotUnique. The second lookup finds the row the other request created and updates it, returning 200 like any re-registration.Not changed: reassigning a token to the caller. That is the documented behavior for a device moving to whoever signs in on it. Pushes are delivered to the device (the token), so someone who obtained another person's token could only send their own notifications to that device and stop that person's. They could not read the other person's notifications, which corrects what the earlier review said.
Verification
RecordNotUnique.after_teardown, after the fixture transaction rolls back. My first version deleted the row insideensurewhile the test transaction still held its lock, and it hung; the hung runs were killed and no rows were left behind.RecordNotUniqueback.bin/rails test: 505 runs, 0 failures. rubocop and brakeman are clean. The AGENTS.md test count is updated.🤖 Generated with Claude Code
https://claude.ai/code/session_01L4Pma9VAqB5A22KCEEin9R