Skip to content

fix: to handle 429 responses more gracefully.#5

Merged
chrisdoehring merged 14 commits intomainfrom
release-20250909-hotfix
Mar 3, 2026
Merged

fix: to handle 429 responses more gracefully.#5
chrisdoehring merged 14 commits intomainfrom
release-20250909-hotfix

Conversation

@chrisdoehring
Copy link
Copy Markdown

This pull request introduces improved handling and testing for API rate limits (HTTP 429) in the Ctrack Crystal integration, as well as several enhancements to the development environment and reliability. The main focus is on robustly managing rate limit responses, ensuring that retries are handled correctly, and that errors are surfaced and logged appropriately. Additionally, more comprehensive tests have been added, and the local development Docker setup has been improved.

API Rate Limit Handling and Error Management:

  • Improved the _get_retry_after function in client.py to correctly parse both integer and HTTP-date Retry-After headers, enforce a minimum wait time, and avoid immediate retry storms.
  • Updated all relevant action handlers (such as action_auth, action_pull_observations, action_trigger_fetch_vehicle_observations, and action_fetch_vehicle_trips in handlers.py) to catch CTCTooManyRequestsException, log warnings, and return clear error responses or propagate the exception as appropriate. [1] [2] [3] [4]

Testing Enhancements:

  • Added and expanded tests in test_client.py and test_handlers.py to cover rate-limiting scenarios, including various Retry-After header formats and error propagation through the handler stack. [1] [2] [3] [4]
  • Mocked asyncio.sleep in tests to avoid unnecessary delays when simulating backoff and retry logic. [1] [2]

Development and Docker Improvements:

  • Added a web-ui service to docker-compose.yml for easier local frontend development, and improved healthchecks for faster and more reliable startup. [1] [2]

Dependency Updates:

  • Added marshmallow to requirements.in to ensure necessary serialization/deserialization support.

Other Minor Improvements:

  • Added missing imports and docstrings for clarity and maintainability. [1] [2] [3]

These changes collectively make the integration more robust against rate limits, improve observability and error handling, and enhance the local development experience.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the Ctrack Crystal integration’s behavior under API rate limiting (HTTP 429), adds tests around those scenarios, and updates local development tooling (Docker + dependency compilation).

Changes:

  • Enhance 429 retry behavior by parsing Retry-After (seconds and HTTP-date) and logging backoff attempts.
  • Add/extend handler and client tests for 429 scenarios and mock asyncio.sleep to keep tests fast.
  • Update local Docker Compose and dependency files (including adding marshmallow to integration deps and regenerating requirements.txt).

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
requirements.txt Regenerated lockfile via uv, now includes dev/test dependencies.
requirements.in Adds marshmallow constraint for integration-specific deps.
local/docker-compose.yml Adds web-ui service, tweaks healthcheck, pins emulator platform.
app/actions/tests/test_handlers.py Adds 429-focused handler tests.
app/actions/tests/test_client.py Adds _get_retry_after tests and mocks sleep for 429 backoff.
app/actions/handlers.py Adds 429 handling in actions, adds crontab schedule decorator, adjusts trigger behavior.
app/actions/client.py Improves Retry-After parsing and adds 429 backoff logging.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread requirements.in Outdated
Comment thread local/docker-compose.yml

web-ui:
build:
context: ./web-ui
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The new web-ui service build context points to ./web-ui relative to local/docker-compose.yml, but there is no local/web-ui directory in the repository. This will cause docker compose up to fail when it tries to build the service. Either add the missing directory/Dockerfile, or adjust/remove the service definition to reference an existing path/image.

Suggested change
context: ./web-ui
context: ../web-ui

Copilot uses AI. Check for mistakes.
Comment thread app/actions/handlers.py Outdated
Comment thread app/actions/handlers.py
Comment on lines 206 to 207
logger.error(f"Failed to process vehicles from integration ID {integration.id}, username: {auth_config.username}")
raise e
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

except Exception as e: ... raise e discards the original traceback context. Use a bare raise here to preserve the stack trace (and consider logger.exception(...) so the exception info is logged automatically).

Suggested change
logger.error(f"Failed to process vehicles from integration ID {integration.id}, username: {auth_config.username}")
raise e
logger.exception(f"Failed to process vehicles from integration ID {integration.id}, username: {auth_config.username}")
raise

Copilot uses AI. Check for mistakes.
Comment thread app/actions/tests/test_client.py
Occasionally we'll see 502 errors when ctrack is under maintenance.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@chrisdoehring chrisdoehring merged commit 9baffff into main Mar 3, 2026
3 checks passed
@chrisdoehring chrisdoehring deleted the release-20250909-hotfix branch March 3, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants