Skip to content

Fix successful request retry returning previous failure - #356

Merged
iLLiCiTiT merged 5 commits into
developfrom
bugfix/rest-retry-returns-previous-failure
Sep 14, 2026
Merged

iLLiCiTiT merged 5 commits into
developfrom
bugfix/rest-retry-returns-previous-failure

Conversation

@BigRoy

@BigRoy BigRoy commented Sep 12, 2026

Copy link
Copy Markdown
Member

Bug

Retries of REST requests can't succeed. If an attempt fails (connection error, timeout, 502, 503) and a later retry succeeds, _do_rest_request still returns the first failure.

This affects every get/post/put/... call and GraphQl queries, e.g. while the server is restarting or behind a flaky proxy.

Cause

The failure of an attempt is stored in new_response. It was never cleared, and after the retry loop:

if new_response is not None:
    return new_response

so the successful response from a later attempt was ignored.

Fix

Reset new_response = None at the start of each attempt, so only a failure of the last attempt is returned.

Reproduce

Needs an attempt to fail and the next one to succeed, so the easiest way is with a patched request function:

import requests
import ayon_api

con = ayon_api.get_server_api_connection()
orig_get = con._session_functions_mapping[ayon_api.utils.RequestTypes.get]
calls = []

def flaky_get(url, **kwargs):
    calls.append(url)
    if len(calls) == 1:
        raise requests.exceptions.ConnectionError("simulated")
    return orig_get(url, **kwargs)

con._session_functions_mapping[ayon_api.utils.RequestTypes.get] = flaky_get
response = con.get("projects")
print(len(calls), response.status_code)
# develop: 2 500   (second attempt succeeded, first failure returned)
# this PR: 2 200

Testing notes

  • tests/test_request_retries.py covers a connection error and a 503 followed by success.
  • Manual: restart the server while a script polls con.get("info"); requests that succeed on a retry now return data instead of an error.

🤖 Generated with Claude Code

BigRoy and others added 2 commits September 13, 2026 00:05
'_do_rest_request' stored the error response of a failed attempt
(connection error, timeout, 502, 503) and never cleared it. After the
retry loop the stored failure was returned even when a later attempt
succeeded. Reset it at the start of each attempt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@BigRoy
BigRoy requested a lite review from Copilot September 14, 2026 09:16
Comment thread ayon_api/server_api.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved review issues were identified.

Pull request overview

Fixes request retries so a later successful attempt is returned instead of an earlier failure.

Changes:

  • Clears stale failure state before each retry.
  • Adds regression tests for connection errors and HTTP 503 responses.
  • Adds a reusable fake response helper.
File summaries
File Description
tests/test_request_retries.py Verifies successful retries return successful responses.
tests/fake_transfer.py Provides a minimal response test double.
ayon_api/server_api.py Resets retry failure state before each attempt.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@BigRoy
BigRoy marked this pull request as ready for review September 14, 2026 14:23
@BigRoy
BigRoy requested review from iLLiCiTiT and a lite review from Copilot September 14, 2026 14:23
@BigRoy BigRoy added the type: bug Something isn't working label Sep 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

Retry handling is corrected and covered by targeted tests.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@BigRoy BigRoy self-assigned this Sep 14, 2026
@iLLiCiTiT
iLLiCiTiT merged commit 49a9aa4 into develop Sep 14, 2026
3 checks passed
@iLLiCiTiT
iLLiCiTiT deleted the bugfix/rest-retry-returns-previous-failure branch September 14, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants