feat: add retry mechanism and configurable interval for update checks #201
Conversation
Unit Test Results10 tests 10 ✅ 1s ⏱️ Results for commit 5463e2e. ♻️ This comment has been updated with latest results. |
src/main/java/net/onelitefeather/antiredstoneclockremastered/service/UpdateService.java
Outdated
Show resolved
Hide resolved
theEvilReaper
left a comment
theEvilReaper
left a comment
There was a problem hiding this comment.
Just a small thing from my side
There was a problem hiding this comment.
Pull Request Overview
This PR adds retry logic and configurable interval for the update check service. The changes introduce a retry mechanism that stops checking for updates after a maximum number of failed attempts and makes the update check interval configurable via system properties.
- Adds configurable update check interval via system property
ARCR_UPDATE_SERVICE_SCHEDULE - Implements retry count mechanism with configurable max retries via system property
ARCR_UPDATE_SERVICE_MAX_RETRY - Adds HTTP status code validation to detect and count failed update checks
Comments suppressed due to low confidence (1)
src/main/java/net/onelitefeather/antiredstoneclockremastered/service/UpdateService.java:106
- When an IOException or InterruptedException occurs, the retry count is not incremented, but it is incremented for non-200 status codes. This inconsistency means network failures (exceptions) don't count toward the max retry limit, while HTTP errors do. Consider incrementing
retryCount++in the catch block to ensure all failures are counted consistently.
} catch (IOException | InterruptedException e) {
LOGGER.error("Something went wrong to check updates", e);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/net/onelitefeather/antiredstoneclockremastered/service/UpdateService.java
Outdated
Show resolved
Hide resolved
| private final SchedulerService schedulerService; | ||
| private Version remoteVersion; | ||
| private ScheduledTask scheduler; | ||
| private int retryCount = 0; |
There was a problem hiding this comment.
The retryCount field is accessed from the asynchronous scheduled task without synchronization. Multiple scheduled tasks could potentially execute concurrently, leading to race conditions when reading and incrementing retryCount. Consider using AtomicInteger instead of int for thread-safe operations.
There was a problem hiding this comment.
What would be the alternative for it ?
src/main/java/net/onelitefeather/antiredstoneclockremastered/service/UpdateService.java
Show resolved
Hide resolved
|
@TheMeinerLP check the copilot feedback please |
# [2.8.0](v2.7.2...v2.8.0) (2025-11-22) ### Features * add retry mechanism and configurable interval for update checks ([#201](#201)) ([12c104f](12c104f))
|
🎉 This PR is included in version 2.8.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Proposed changes
We added a status code check and also a retry config to improve stability.
Fixes #196
Types of changes
What types of changes does your code introduce to this project?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any ofthem, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before
merging your code.
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you
did and what alternatives you considered, etc...