fix: handle excessively long URLs by performing local equality checks. ref #7986#8432
fix: handle excessively long URLs by performing local equality checks. ref #7986#8432len360 wants to merge 2 commits intopubkey:masterfrom
Conversation
On patch event: 1. Fetch the selected document from the server 2. Compare the assumed master state with the server state locally 3. If there is a difference -> conflict detected, return the server state 4. Otherwise -> update the server state
|
Hi @pubkey, I'm embarrassed because the issue detected in Another thing is that the issue in |
| query | ||
| ); | ||
| // fetch the current document state from the server | ||
| const docOnServer: WithDeleted<RxDocType> = await fetchById(id); |
There was a problem hiding this comment.
By fetching here first it is not inside of one transaction anymore. What happens if someone else changes the document on the server after we fetch docOnServer and before sending the new one to the server?
There was a problem hiding this comment.
In fact, this implementation seems problematic. If someone modifies the data in the database while another user is fetching it, the last user to write will overwrite the previous changes without any conflict handling.
Another way to address the issue of overly long URLs would be to add a hash property to the object. This "hash" could be included in the URL alongside the identifier, allowing the update to be performed in a single transaction while also helping detect potential conflicts.
What do you think about this approach?
2 participants
This PR contains:
Describe the problem you have without this PR
Without this fix, large payloads were being included in the URL, causing bad request errors. This issue is related to supabase/postgrest-js#393.
Pull Request Description
This PR contains a bug fix corresponding to issue #7986. It removes the inclusion of all parameters in the URL in order to perform a local comparison of objects instead of making long requests that cause bad request errors.
Logic implemented
On update events:
Unit tests
A unit test has been added to verify that updating a property with a large number of characters does not cause any errors, ensuring that this pull request properly resolves the issue.