Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

feat: Add requests idempotency verification on the server side #400

Merged
merged 3 commits into from Mar 30, 2020

Conversation

mykola-mokhnach
Copy link
Contributor

@mykola-mokhnach mykola-mokhnach commented Mar 28, 2020

The idea behind that is to be able to properly handle client side retries.

How this is supposed to work:

  • The client adds a randomly generated X-Idempotency-Key header to each request we'd like to make idempotent (each request must have a different value). The server itself only considers header presence in POST and PUT requests.
  • The server temporarily stores the response stream of the marked request in the internal LRU cache. After the request is done then its result is serialised to the file system.
  • If the second request with an idempotency key is coming and no matches are found then the request proceeds further
  • If there is an active request with the same key then the response stream of that request will be rerouted to the current one, so the client would receive the same response
  • If there was a request with the same key then the previously serialized cached response would be piped to the current response
  • All cached responses are removed from the cache automatically as soon as they expire or the cache reaches its limit, since its a LRU cache type

Read https://stripe.com/docs/api/idempotent_requests for more details on this topic

@mykola-mokhnach
Copy link
Contributor Author

This change will require to tune the clients, although I think the improvement worth it

@mykola-mokhnach
Copy link
Contributor Author

cc @ki4070ma

@mykola-mokhnach mykola-mokhnach changed the title feat: Add requests idempotency verification on the server side [WIP] feat: Add requests idempotency verification on the server side Mar 28, 2020
@jlipps
Copy link
Member

jlipps commented Mar 28, 2020

Can you first help me understand in which scenarios we want client side retries?

@mykola-mokhnach
Copy link
Contributor Author

@jlipps We don't control it. Client libraries, like OkHttp (the one that java client uses), apply retries automatically. See https://square.github.io/okhttp/4.x/okhttp/okhttp3/-ok-http-client/-builder/retry-on-connection-failure/ for more details

@mykola-mokhnach mykola-mokhnach changed the title [WIP] feat: Add requests idempotency verification on the server side feat: Add requests idempotency verification on the server side Mar 30, 2020
Copy link
Contributor

@imurchie imurchie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. I do think that all the unnecessary fat-arrow functions will make stack traces less followable.

package.json Outdated Show resolved Hide resolved
lib/express/idempotency.js Outdated Show resolved Hide resolved
@mykola-mokhnach mykola-mokhnach merged commit 0cee519 into appium:master Mar 30, 2020
@mykola-mokhnach mykola-mokhnach deleted the idemp branch March 30, 2020 20:19
@KazuCocoa
Copy link
Member

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants