feedback on draft-idempotency-header

[olijaun]

Hi

It's great to see this RFC. Thanks for your work! I have three points I'd like to discuss:

1. In 2.6 you write: "Retry The request was retried after the original request completed. The resource server MUST respond with the result of the previously completed operation, success or an error." This makes of course sense -- especially if you consider the "mathematical definition" of idempotence. However for my taste "MUST" is a bit too restricting. Technically all you need to know as a client is whether a request has been processed before or not. So from a technical perspective it is sufficient if a service just returns an error code indicating that the request has been processed before. This might be much simpler to implement. Imagine that you have an application where you only manage the "current state" of your data. If a POST request returns the newly created resource then you need to know what the state of this resource was when it was created. This is not possible in a simple CRUD-Application that only tracks the current state.
2. In section 2.7 you write "If there is an attempt to reuse an idempotency key with a different request payload, the resource server MUST reply with a "HTTP" "422" status code with body containing a link pointing to relevant documentation." I think this is too restrictive. We often only check the Idempotency-Key and not the payload. Why? It is the responsibility of the client to make sure that it always sends the same payload for a given Idempotency-Key. It is sufficient if the backend only checks whether the given Idempotency-Key has been processed. It might of course be helpful for a client if the backend does perform such a check but it is not really required technically. Of course you would have to document the exact behavior so that clients know exactly what to expect. But the same is the case for the "expiration policy" you are mentioning in the RFC.
3. As you may have noticed my mother tongue is not English (it is Swiss German). I looked-up the term "Idempotency" in my favorite dictionary (dict.leo.org) and it does not exist. However there exists the term "Idempotence". Also in the Introduction you say that "Idempotence is the property of certain operations in mathematics". So… why Idempotency-Key and not Idempotence-Key? I'm of course completely unqualified for this kind of question 😊 Just a thought…

Thanks and kind regards Oliver Jaun

[jayadeba]

@olijaun

Happy to discuss these. Few comments below

1- The write-up is more about returning the exact processing state of the idempotent request if it is already processed. A request is either successfully processed or failed. Successful processing means a 200 response with the current resource state and a failed response usually is a 4XX/5XX response with error details. A Retry mostly takes place for a scenario when the connection to the client is lost before the server returns (or writes) the completed processing state of the request, which means the server should always emit the completed processing state (the exact status of the request processing, with an exception for few recoverable errors). Pls note that this is a different scenario than the "Request In Flight" (i.e. Request processing is in progress and the server receives a duplicate request- In this case returning an error code (409) would be fine and the write-up describes this scenario).

2-Idempotence Contract= Key + Fingerprint. Key is only used for lookup the payload in the idempotence cache (The request payload is then processed idempotently.). If only the key is used to check the idempotent condition, when a client reuses a previously used key for a new payload, the idempotent implementation would wrongly return the response status of the old request payload for which the key was previously used (wrong behavior) VS when both key + fingerprint are used as the contract, it is an error condition and the server responds with an errror, which is the correct behavior. What are we suggesting here? Are we saying that we soften the language and use SHOULD instead of MUST?

3- I'm good with Idempotence-Key if that is the right key name from English language standpoint but there are already widespread usage of Idempotency-Key in public APIs:-)

[olijaun]

@jayadeba

Thanks for your answer. Just to be clear: I'm not talking about any mistakes in the spec. It's - as you say - about softening the language and use SHOULD instead of MUST. I could of course always use a proprietary header for my purpose. So it is up to you what "softness" you are willing to accept :-)

I get your point and it absolutely makes sense. I also get the reason of the 409 and I wouldn't change anything about the 409-case. I'll give you an concrete example to explain my point: Imagine a simple "Person-Resource". You create a new person by posting a JSON:

POST /persons
Idempotency-Key: 1a2f0bd1-2a19-4b89-ae19-818562005d78

{
    "name": "John"
}

<client has a timeout (or has lost connection) and does NOT receive the following response:>

200 OK
Content-Type: application/json

{
    "id": "de2e865b-d66c-4f88-878d-071c1514c10b",
    "name": "John"
}

Now assume another client changes the "name" from "John" to "Bob" using PUT or PATCH.

Now the client that previously had a timeout retries the request:

POST /persons
Idempotency-Key: 1a2f0bd1-2a19-4b89-ae19-818562005d78

{
    "name": "John"
}

200 OK
Content-Type: application/json

{
    "id": "de2e865b-d66c-4f88-878d-071c1514c10b",
    "name": "John" // the name in the database is now "Bob" but we have to return "John"
}

What I understand from the RFC is that it MUST return "John" here although the name has been changed to "Bob" in the meantime.

As I said before: this makes completely sense but is not trivial to be implemented in a simple CRUD-Application that just stores the "current state" in the database.

Therefore I suggest to introduce the possibility to alternatively respond with a 2xx-StatusCode other than 200 to indicate to the client that the request with the given Idempotency-Key has been (successfully) processed already (maybe a 204 with a link to "idempotency"?). This means of course a bit more complexity on the client side and a bit less complexity on the server side.

Regarding 2): yes, I'm suggesting to soften the language and make the "Fingerprint" an optional part of the Idempotency-Contract.

As I said before: I see that this is somehow a contradiction to the term "Idempotence". My reasoning is purely pragmatic.

[jayadeba]

@olijaun Good points. Thanks for the explanation.

1-In section 2.6, it makes sense to soften the language from MUST to MAY or SHOULD. Receiving a stale resource representation is ok for many clients. If a client can't tolerate a stale representation, it can always fetch the latest resource representation by doing a subsequent GET. Event with a GET, on the server-side, in the distributed architecture, there's as well no guarantee that the client would receive a latest representation. Also returning the earlier processed response in case of a retry makes things simpler to implement, if someone wants to implement request idempotence as a common edge service at the API gateway layer. Any 2XX response is ok for this scenario (although, for resource creation POST operations, 201 with resource link or 200 with the complete resource representation are preferred)

2-Regarding 2.7, IMO, making idempotency Fingerprint optional MAY weaken the idempotence contract & condition. But we are open to hearing more views on this and we can eventually decide the suitable language.

[jayadeba]

@olijaun We have softened the language to SHOULD for the behavior that the server MAY respond with the latest resource if the resource was modified by another client while the original client's request timed out. So this issue is resolved.

We also made idenpotency key optional (An idempotency fingerprint MAY be used in conjunction with an idempotency key to determine the uniqueness of a request.).

Resolving the issue.