Skip to content

feat: restored retrying of rate-limited Leantime requests - #327

Open
tuj wants to merge 4 commits into
feature/leantime-sync-throwable-handlingfrom
feature/leantime-sync-429-retry
Open

feat: restored retrying of rate-limited Leantime requests#327
tuj wants to merge 4 commits into
feature/leantime-sync-throwable-handlingfrom
feature/leantime-sync-429-retry

Conversation

@tuj

@tuj tuj commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Link to ticket

https://leantime.itkdev.dk/?tab=ticketdetails#/tickets/showTicket/8000

Description

  • Restored retrying of rate-limited Leantime requests. RetryableHttpClient with a 429 retry strategy was added in b64773db ("1595: Added retryable http client to handle rate limiting") and lost in b27ba16e when the Jira stack was removed, leaving docker-compose.server.override.yml still commenting that the sync is rate limited by the Leantime API. A 429 surfaces inside updateAsJob() before the next page is queued, so a single one ended the whole pagination chain. LeantimeApiService now gets a retrying client via app.leantime.http_client, tunable with APP_HTTP_CLIENT_RETRY_DELAY_MS and APP_HTTP_CLIENT_MAX_RETRIES. The retried status codes are a flat list, because GenericRetryStrategy's defaults restrict transport errors and 5xx to idempotent methods — which excludes the POSTs the Leantime data API uses even for reads.
  • Added timeout: 30 and max_duration: 300 to framework.http_client.default_options. Symfony caps neither by default, so a Leantime instance that accepted a connection and then stalled held the messenger worker indefinitely — --time-limit is only checked between messages, never during one. max_duration has to clear the unpaginated /deleted endpoint, which returns a week of history in one response for sync-deleted --interval=P1W.

Checklist

  • My code is covered by test cases.
  • My code passes our test (all our tests).
  • My code passes our static analysis suite.
  • My code passes our continuous integration process.

tuj added 2 commits July 31, 2026 08:55
fetchFromLeantime() calls getContent() with default error handling, so a 429
throws out of updateAsJob() before the next page is queued — one rate-limited
response ended the whole pagination chain. With five staggered nightly syncs
and a 15-minute incremental cron against the same API, that is the likeliest
halt in production.

RetryableHttpClient with a 429 strategy was added for exactly this in b64773d
and deleted in b27ba16 with the old Jira stack, leaving
docker-compose.server.override.yml still commenting that the sync is rate
limited by the Leantime API. This restores it as app.leantime.http_client,
injected only into LeantimeApiService so the monitoring ping keeps the plain
client, and tunable via APP_HTTP_CLIENT_RETRY_DELAY_MS and
APP_HTTP_CLIENT_MAX_RETRIES.

The retried status codes are a flat list rather than GenericRetryStrategy's
defaults: those express transport errors and 500/504/507/510 as
[code => idempotent methods], which excludes POST, and the Leantime data API
uses POST even for reads.

Also sets timeout and max_duration on the default HTTP client. Symfony caps
neither, so a Leantime instance that accepted a connection and then stalled
held the worker indefinitely — messenger's --time-limit is only checked
between messages. max_duration is 300s to clear the unpaginated /deleted
endpoint used by sync-deleted --interval=P1W.
@tuj tuj self-assigned this Jul 31, 2026
@tuj tuj added the bug Something isn't working label Jul 31, 2026
@tuj
tuj requested a review from turegjorup August 4, 2026 08:43

@turegjorup turegjorup left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is the wrong direction for retries. I think the message handlers should differentiate between exceptions and only throw UnrecoverableMessageHandlingException when it actually makes sense. so NOT for HTTP errors. Then configure retry on the message queue.

Retry on the http client makes sense when running synchronous. When running async in job queues I would let retry on the http client be disabled (which is default: https://symfony.com/doc/current/reference/configuration/framework.html#enabled-9). And let the message handling control retry.

# has to clear the /deleted endpoint, which the Leantime plugin does not paginate, so
# `sync-deleted --interval=P1W` pulls a week of history in one response.
timeout: 30
max_duration: 300

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

30 is high for timeout. I would go for 5
300 for max_duration also high. If the deletedendpoint performs this badly it should be paginated. As an alternative configure two http clients. One with normal, thight defaults, one with longer timeouts for the deleted endpoints.

Make scoped clients, so you can configure them individually, don't alter and depend on defaults.

Use the rate limiter: https://symfony.com/doc/current/http_client.html#limit-the-number-of-requests and configure it to match Leantime, maybe with a little headroom

Comment thread config/services.yaml
$client: "@http_client"
$strategy: '@Symfony\Component\HttpClient\Retry\RetryStrategyInterface'
$maxRetries: "%env(int:APP_HTTP_CLIENT_MAX_RETRIES)%"
$logger: "@logger"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Set shorter timeout here, use the rate_limiter option: https://symfony.com/doc/current/http_client.html#limit-the-number-of-requests

Comment thread config/services.yaml
$statusCodes: [0, 423, 425, 429, 500, 502, 503, 504, 507, 510]
$delayMs: "%env(int:APP_HTTP_CLIENT_RETRY_DELAY_MS)%"
$multiplier: 2.0
# RetryableHttpClient prefers the response's Retry-After header over this backoff, so the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you validated that Leantime (or our leantime plugin) even sends a Retry-After header?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants