Skip to content
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.

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

Change PlaybackSession createFromOld to use upsert instead of create #3436

Merged

Conversation

mikiher
Copy link
Contributor

@mikiher mikiher commented Sep 18, 2024

This fixes #2662

While I'm not in the position to reproduce this (I don't own an iPhone), I'm pretty sure why the crash happens.

It is very likely due to duplicate /session/local requests (retries?) sent in rapid succession from the client and handled asynchronously on the server, causing a race condition.

While Request A is awaiting

let session = await Database.getPlaybackSession(sessionJson.id)

Request B is also making the same call.
both receive a null response, so each of them calls create() on the PlaybackSession model, and one of them succeeds while the other fails with UniqueConstraintError because it is trying to create a row with the same primary key.

The fix is simple - since the requests are likely identical, it is safe to use upsert instead of create, which will succeed for both A and B (this of course doesn't prevent the race condition - doing that would involve using transactions, which I think is an overkill in this case)

@mikiher mikiher marked this pull request as ready for review September 18, 2024 15:34
@mikiher
Copy link
Contributor Author

mikiher commented Sep 18, 2024

Also added a small unrelated change to logging of unhandled rejections, so that the full error object is printed, not just the error message.

@advplyr
Copy link
Owner

advplyr commented Sep 18, 2024

Yeah the local sync endpoint is susceptible to a race condition. The regular sync is not since creating the session and syncing the session are separate requests.

Thanks!

@advplyr advplyr merged commit 013c7c7 into advplyr:master Sep 18, 2024
5 checks passed
@mikiher mikiher deleted the create-playback-session-race-condition branch November 18, 2024 09:40
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.

[Bug]: Server crash when iOS app connects to server, unique constraint violation -
2 participants