pact-foundation / pact-jvm

JVM version of Pact. Enables consumer driven contract testing, providing a mock service and DSL for the consumer project, and interaction playback and verification for the service provider project.
https://docs.pact.io
Apache License 2.0
1.08k stars 479 forks source link

HTTP Client does not retry all types of requests #1710

Closed mefellows closed 10 months ago

mefellows commented 1 year ago

When the broker errors with a 5xx status code the HTTP client should attempt, up to a configurable limit, to retry the request.

We currently use the DefaultHttpRequestRetryStrategy here: https://github.com/pact-foundation/pact-jvm/blob/2b2055f0d9e0d10c2bfe51898d7163e037a772d9/core/support/src/main/kotlin/au/com/dius/pact/core/support/HttpClient.kt#L89C49-L89C49, however this strategy ignores potentially non-idempotent requests. So all POST, PUT, DELETE etc. will not be retried.

This includes local verification which takes a body for the "pacts for verification" endpoint.

From the docs on that class: https://hc.apache.org/httpcomponents-client-4.5.x/current/tutorial/html/fundamentals.html#d5e293

By default HttpClient assumes only non-entity enclosing methods such as GET and HEAD to be idempotent and entity enclosing methods such as POST and PUT to be not for compatibility reasons.

The Pact Broker assumes retries because it's designed for CI/CD systems where builds can be re-run multiple times, so it is safe to retry all requests.

Discovered via this slack conversation: https://pact-foundation.slack.com/archives/CLS16AVEE/p1692695354797019.

github-actions[bot] commented 1 year ago

👋 Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-1286). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.