Skip to content

feat: add retry mechanism and configurable interval for update checks #201

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: add retry mechanism and configurable interval for update checks #201

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6de88cb
feat: add retry mechanism and configurable interval for update checks
TheMeinerLP Nov 3, 2025
fc004dc
fix: change UPDATE_CHECK_INTERVAL_TICKS to use lowercase 'long' type
TheMeinerLP Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/main/java/net/onelitefeather/antiredstoneclockremastered/service/UpdateService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@
@Singleton
public final class UpdateService implements Runnable {
private static final Logger LOGGER = LoggerFactory.getLogger(UpdateService.class);
private static final long UPDATE_CHECK_INTERVAL_TICKS = Long.getLong("ARCR_UPDATE_SERVICE_SCHEDULE", 20 * 60 * 60 * 3L); // 3 hours
private static final int MAX_RETRY_COUNT = Integer.getInteger("ARCR_UPDATE_SERVICE_MAX_RETRY", 5);
private static final String DOWNLOAD_URL = "https://hangar.papermc.io/OneLiteFeather/AntiRedstoneClock-Remastered/versions/%s";
private final HttpClient hangarClient = HttpClient.newBuilder().build();
private final Version localVersion;
private final SchedulerService schedulerService;
private Version remoteVersion;
private ScheduledTask scheduler;
private int retryCount = 0;
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

@Inject
public UpdateService(AntiRedstoneClockRemastered antiRedstoneClockRemastered, SchedulerService schedulerService) {
Expand All @@ -38,6 +41,12 @@ public UpdateService(AntiRedstoneClockRemastered antiRedstoneClockRemastered, Sc

@Override
public void run() {
if (retryCount >= MAX_RETRY_COUNT) {
LOGGER.error("Max retry count reached for update check, stopping further attempts");
LOGGER.error("Please check your internet connection or https://hangar.papermc.io/ status.");

Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The early return when max retry count is reached prevents the scheduled task from continuing. However, the task will continue to run on its schedule even after max retries are reached, causing this code block to execute repeatedly. Consider canceling the scheduler when max retries are reached using this.scheduler.cancel() to prevent unnecessary repeated checks.

Suggested change
if (this.scheduler != null) {
this.scheduler.cancel();
}

Copilot uses AI. Check for mistakes.
return;
}
var remoteVersion = getNewerVersion();
if (remoteVersion != null) {
this.remoteVersion = remoteVersion;
Expand Down Expand Up @@ -76,14 +85,19 @@ private void notifyPlayer(Version localVersion, Version remoteVersion, Player pl
}

public void schedule() {
this.scheduler = schedulerService.runTaskTimerAsynchronously(scheduledTask -> this.run(), 0, 20 * 60 * 60 * 3);
this.scheduler = schedulerService.runTaskTimerAsynchronously(scheduledTask -> this.run(), 0, UPDATE_CHECK_INTERVAL_TICKS);
}


@Nullable
private Version getNewerVersion() {
try {
HttpResponse<String> httpResponse = hangarClient.send(Constants.LATEST_RELEASE_VERSION_REQUEST, HttpResponse.BodyHandlers.ofString());
if (httpResponse.statusCode() != 200) {
LOGGER.error("Failed to check for updates, status code: {}", httpResponse.statusCode());
retryCount++;
return null;
}
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

Successful update checks (status code 200) that don't find a newer version don't reset the retry count. This means if the service experiences 4 failures followed by repeated successful checks that find no updates, it will stop working after one more failure. Consider resetting retryCount = 0 when a successful response is received (status code 200) to reset the failure counter.

Suggested change
}
}
retryCount = 0;

Copilot uses AI. Check for mistakes.
Version remoteVersion = Version.parse(httpResponse.body());
if (remoteVersion.isHigherThan(this.localVersion)) {
return remoteVersion;
Expand Down