Return 409 Conflict on concurrent update errors

[guewen]

Currently, when a concurrent update error happens (the famous could not serialize access due to concurrent update), the endpoints return an HTTP error code 500 Internal Server Error.

Odoo RPC handles retries on such errors such that a client gets this error only after all the tries have failed, which could lead to a long delay before getting a response.

@TDu proposed to return 409 Conflict on such errors. I really like it, because then it leaves control to the client application to decide if and when they retry.

Should I propose a PR?

From https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.10

409 Conflict

The request could not be completed due to a conflict with the current state of the resource. This code is only allowed in situations where it is expected that the user might be able to resolve the conflict and resubmit the request. The response body SHOULD include enough information for the user to recognize the source of the conflict. Ideally, the response entity would include enough information for the user or user agent to fix the problem; however, that might not be possible and is not required.

Conflicts are most likely to occur in response to a PUT request. For example, if versioning were being used and the entity being PUT included changes to a resource which conflict with those made by an earlier (third-party) request, the server might use the 409 response to indicate that it can't complete the request. In this case, the response entity would likely contain a list of the differences between the two versions in a format defined by the response Content-Type.

[lmignon]

@guewen Good Idea!

[etobella]

Good idea @guewen

[guewen]

My initial premise was wrong: I thought that REST endpoint did not have the retry mechanism of odoo, but after tests, actually they do. The retry is done above in the stack.

It doesn't mean we shouldn't change, but we have several ways to tackle it. And I'm not sure that I'll be able to work on any of them because the current retry may be "good enough".

1. Remove the core's retry for REST endpoint and return 409 conflict on the first error: gives more predictable results (at least regarding duration of requests), but it forces clients to handle it (so it may appear as a regression)
2. Keep the core's retry and return a 409 only when the request did not pass after all the retries

[github-actions[bot]]

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.

[gurneyalex]

I'd like to reopen this issue as I'm getting bitten by the lack of retry in base_rest on a customer project where the customer gets the 500 http status in his API calls and complains about it.

@sbidoul @lmignon is there a specific reason why there is no handling of concurrent updates in base_rest? (in other words, would a PR adding this be rejected?)

[simahawk]

My POV on 409: seems a good solution for certain cases, but should be optional IMO. I would improve https://github.com/OCA/rest-framework/blob/14.0/base_rest/http.py#L176 to delegate the handling to ir.http or another helper model so that you can easily override it on per project base w/out patching.

[github-actions[bot]]

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.