github and cf: improve detection and logging of rate limiting (#401)

itzg · web-flow · commit fded16b03a8a · 2024-02-18T14:42:19.000-06:00
diff --git a/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java b/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java
@@ -40,6 +40,7 @@
 import me.itzg.helpers.curseforge.model.ModLoader;
 import me.itzg.helpers.errors.GenericException;
 import me.itzg.helpers.errors.InvalidParameterException;
+import me.itzg.helpers.errors.RateLimitException;
 import me.itzg.helpers.fabric.FabricLauncherInstaller;
 import me.itzg.helpers.files.Manifests;
 import me.itzg.helpers.files.ResultsFileWriter;
@@ -70,10 +71,12 @@ public class CurseForgeInstaller {
     private final Path outputDir;
     private final Path resultsFile;
 
-    @Getter @Setter
+    @Getter
+    @Setter
     private String apiBaseUrl = "https://api.curseforge.com";
 
-    @Getter @Setter
+    @Getter
+    @Setter
     private String apiKey;
 
     @Getter
@@ -157,7 +160,7 @@ public void installFromModpackManifest(String modpackManifestLoc, String slug) t
 
             processModpackManifest(context, modpackManifest,
                 () -> new Result(Collections.emptyList(), null)
-                );
+            );
         });
     }
 
@@ -183,14 +186,15 @@ void install(String slug, InstallationEntryPoint entryPoint) throws IOException
         if (apiKey == null || apiKey.isEmpty()) {
             if (manifest != null) {
                 log.warn("API key is not set, so will re-use previous modpack installation of {}",
-                    manifest.getSlug() != null ? manifest.getSlug() : "Project ID "+manifest.getModId());
+                    manifest.getSlug() != null ? manifest.getSlug() : "Project ID " + manifest.getModId()
+                );
                 log.warn("Obtain an API key from " + ETERNAL_DEVELOPER_CONSOLE_URL
-                    + " and set the environment variable "+ API_KEY_VAR +" in order to restore full functionality.");
+                    + " and set the environment variable " + API_KEY_VAR + " in order to restore full functionality.");
                 return;
             }
             else {
                 throw new InvalidParameterException("API key is not set. Obtain an API key from " + ETERNAL_DEVELOPER_CONSOLE_URL
-                    + " and set the environment variable "+ API_KEY_VAR);
+                    + " and set the environment variable " + API_KEY_VAR);
             }
         }
 
@@ -209,11 +213,19 @@ void install(String slug, InstallationEntryPoint entryPoint) throws IOException
 
         } catch (FailedRequestException e) {
             if (e.getStatusCode() == 403) {
-                throw new InvalidParameterException(String.format("Access to %s is forbidden or rate-limit has been exceeded."
-                        + " Ensure %s is set to a valid API key from %s or allow rate-limit to reset.",
+                log.debug("Failed request details: {}", e.toString());
+
+                if (e.getBody().contains("There might be too much traffic")) {
+                    throw new RateLimitException(null, String.format("Access to %s has been rate-limited.", apiBaseUrl), e);
+                }
+                else {
+                    throw new InvalidParameterException(String.format("Access to %s is forbidden or rate-limit has been exceeded."
+                            + " Ensure %s is set to a valid API key from %s or allow rate-limit to reset.",
                         apiBaseUrl, API_KEY_VAR, ETERNAL_DEVELOPER_CONSOLE_URL
-                ), e);
-            } else {
+                    ), e);
+                }
+            }
+            else {
                 throw e;
             }
         }
@@ -229,13 +241,14 @@ static class InstallContext {
     }
 
     interface InstallationEntryPoint {
+
         void install(InstallContext context) throws IOException;
     }
 
     private void installByRetrievingModpackZip(InstallContext context, String fileMatcher, Integer fileId) throws IOException {
         final CurseForgeMod curseForgeMod = context.cfApi.searchMod(context.slug,
-            context.categoryInfo.getClassIdForSlug(CurseForgeApiClient.CATEGORY_MODPACKS)
-        )
+                context.categoryInfo.getClassIdForSlug(CurseForgeApiClient.CATEGORY_MODPACKS)
+            )
             .block();
 
         resolveModpackFileAndProcess(context, curseForgeMod, fileId, fileMatcher);
@@ -273,12 +286,14 @@ else if (Manifests.allFilesPresent(outputDir, context.prevInstallManifest, ignor
         }
 
         log.info("Installing modpack '{}' version {} from provided modpack zip",
-            modpackName, modpackVersion);
+            modpackName, modpackVersion
+        );
 
         final ModPackResults results = processModpack(context, modpackManifest, overridesApplier);
 
         finalizeResults(context, results,
-            pseudoModId, pseudoFileId, results.getName());
+            pseudoModId, pseudoFileId, results.getName()
+        );
     }
 
     private static boolean matchesPreviousInstall(InstallContext context, int modId, int fileId) {
@@ -362,7 +377,8 @@ else if (Manifests.allFilesPresent(outputDir, context.prevInstallManifest, ignor
         }
 
         log.info("Processing modpack '{}' ({}) @ {}:{}", modFile.getDisplayName(),
-            mod.getSlug(), modFile.getModId(), modFile.getId());
+            mod.getSlug(), modFile.getModId(), modFile.getId()
+        );
 
         if (modpackZip == null) {
             throw new GenericException("Download of modpack zip was empty");
@@ -448,7 +464,8 @@ private void finalizeExistingInstallation(CurseForgeManifest prevManifest) throw
     /**
      * @throws MissingModsException if any mods need to be manually downloaded
      */
-    private void finalizeResults(InstallContext context, ModPackResults results, int modId, int fileId, String displayName) throws IOException {
+    private void finalizeResults(InstallContext context, ModPackResults results, int modId, int fileId, String displayName)
+        throws IOException {
         final CurseForgeManifest newManifest = CurseForgeManifest.builder()
             .modpackName(results.getName())
             .modpackVersion(results.getVersion())
@@ -535,19 +552,19 @@ private ModPackResults processModpack(InstallContext context,
 
                 log.debug("Evaluating projectId={} with exclude={} and forceInclude={}",
                     projectID, exclude, forceInclude
-                    );
+                );
 
                 return Mono.just(forceInclude || !exclude)
                     .flatMap(proceed -> proceed ? Mono.just(true)
                         : context.cfApi.getModInfo(projectID)
                             .map(mod -> {
                                 log.info("Excluding mod file '{}' ({}) due to configuration",
                                     mod.getName(), mod.getSlug()
-                                    );
+                                );
                                 // and filter away
                                 return false;
                             })
-                        );
+                    );
             })
             // ...download and possibly unzip world file
             .flatMap(fileRef ->
@@ -567,7 +584,9 @@ private ModPackResults processModpack(InstallContext context,
         return buildResults(modpackManifest, modLoader, modFiles, overridesResult);
     }
 
-    private ModPackResults buildResults(MinecraftModpackManifest modpackManifest, ModLoader modLoader, List<PathWithInfo> modFiles, Result overridesResult) {
+    private ModPackResults buildResults(MinecraftModpackManifest modpackManifest, ModLoader modLoader,
+        List<PathWithInfo> modFiles, Result overridesResult
+    ) {
         return new ModPackResults()
             .setName(modpackManifest.getName())
             .setVersion(modpackManifest.getVersion())
@@ -722,22 +741,23 @@ else if (category.getSlug().equals("worlds")) {
                                             .setDownloadNeeded(resolveResult.downloadNeeded)
                                             .setModInfo(modInfo)
                                             .setCurseForgeFile(cfFile)
-                                    : extractWorldZip(modInfo, resolveResult.path, outputPaths.getWorldsDir())
+                                        : extractWorldZip(modInfo, resolveResult.path, outputPaths.getWorldsDir())
                                 )
                             : resolvedFileMono
-                            .map(resolveResult ->
-                                new PathWithInfo(resolveResult.path)
-                                    .setDownloadNeeded(resolveResult.downloadNeeded)
-                                    .setModInfo(modInfo)
-                                    .setCurseForgeFile(cfFile)
-                            );
+                                .map(resolveResult ->
+                                    new PathWithInfo(resolveResult.path)
+                                        .setDownloadNeeded(resolveResult.downloadNeeded)
+                                        .setModInfo(modInfo)
+                                        .setCurseForgeFile(cfFile)
+                                );
                     });
             })
             .checkpoint(String.format("Downloading file from modpack %d:%d", projectID, fileID));
     }
 
     @RequiredArgsConstructor
     static class ResolveResult {
+
         final Path path;
         @Setter
         boolean downloadNeeded;
@@ -815,7 +835,7 @@ private PathWithInfo extractWorldZip(CurseForgeMod modInfo, Path zipPath, Path w
                 ZipEntry nextEntry = zipInputStream.getNextEntry();
 
                 if (nextEntry == null || !nextEntry.isDirectory()) {
-                    throw new GenericException("Expected top-level directory in world zip "+zipPath);
+                    throw new GenericException("Expected top-level directory in world zip " + zipPath);
                 }
 
                 // Will replace top diretory with slug name
diff --git a/src/main/java/me/itzg/helpers/errors/ExceptionHandler.java b/src/main/java/me/itzg/helpers/errors/ExceptionHandler.java
@@ -1,7 +1,9 @@
 package me.itzg.helpers.errors;
 
+import java.time.Instant;
 import lombok.extern.slf4j.Slf4j;
 import me.itzg.helpers.McImageHelper;
+import me.itzg.helpers.http.FailedRequestException;
 import picocli.CommandLine;
 import picocli.CommandLine.ExitCode;
 import picocli.CommandLine.IExecutionExceptionHandler;
@@ -11,29 +13,46 @@
 @Slf4j
 public class ExceptionHandler implements IExecutionExceptionHandler {
 
-  private final McImageHelper mcImageHelper;
+    private final McImageHelper mcImageHelper;
 
-  public ExceptionHandler(McImageHelper mcImageHelper) {
-    this.mcImageHelper = mcImageHelper;
-  }
+    public ExceptionHandler(McImageHelper mcImageHelper) {
+        this.mcImageHelper = mcImageHelper;
+    }
+
+    @Override
+    public int handleExecutionException(Exception e, CommandLine commandLine, ParseResult parseResult) {
 
-  @Override
-  public int handleExecutionException(Exception e, CommandLine commandLine,
-      ParseResult parseResult) {
+        if (!mcImageHelper.isSilent()) {
+            if (e instanceof InvalidParameterException) {
+                log.error("Invalid parameter provided for '{}' command: {}", commandLine.getCommandName(), e.getMessage());
+                log.debug("Invalid parameter details", e);
+            }
+            else if (e instanceof FailedRequestException) {
+                logUnexpectedException(e, commandLine);
+                log.debug("Failed request details: {}", e.toString());
+            }
+            else if (e instanceof RateLimitException) {
+                logUnexpectedException(e, commandLine);
+                final RateLimitException rle = (RateLimitException) e;
+                final Instant delayUntil = rle.getDelayUntil();
+                if (delayUntil != null) {
+                    log.warn("Rate limit response recommends waiting until {}", delayUntil);
+                }
+            }
+            else {
+                logUnexpectedException(e, commandLine);
+            }
+        }
 
-    if (!mcImageHelper.isSilent()) {
-      if (e instanceof InvalidParameterException) {
-        log.error("Invalid parameter provided for '{}' command: {}", commandLine.getCommandName(), e.getMessage());
-        log.debug("Invalid parameter details", e);
-      } else {
+        final IExitCodeExceptionMapper mapper = commandLine.getExitCodeExceptionMapper();
+        return mapper != null ? mapper.getExitCode(e) : ExitCode.SOFTWARE;
+    }
+
+    private static void logUnexpectedException(Exception e, CommandLine commandLine) {
         log.error("'{}' command failed. Version is {}",
             commandLine.getCommandName(),
             McImageHelper.getVersion(),
-            e);
-      }
+            e
+        );
     }
-
-    final IExitCodeExceptionMapper mapper = commandLine.getExitCodeExceptionMapper();
-    return mapper != null ? mapper.getExitCode(e) : ExitCode.SOFTWARE;
-  }
 }
diff --git a/src/main/java/me/itzg/helpers/errors/RateLimitException.java b/src/main/java/me/itzg/helpers/errors/RateLimitException.java
@@ -0,0 +1,15 @@
+package me.itzg.helpers.errors;
+
+import java.time.Instant;
+import lombok.Getter;
+
+@Getter
+public class RateLimitException extends RuntimeException {
+
+    private final Instant delayUntil;
+
+    public RateLimitException(Instant delayUntil, String message, Throwable cause) {
+        super(message, cause);
+        this.delayUntil = delayUntil;
+    }
+}
diff --git a/src/main/java/me/itzg/helpers/github/GithubClient.java b/src/main/java/me/itzg/helpers/github/GithubClient.java
@@ -1,11 +1,18 @@
 package me.itzg.helpers.github;
 
+import static io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN;
+import static io.netty.handler.codec.http.HttpResponseStatus.TOO_MANY_REQUESTS;
+
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.HttpResponseStatus;
 import java.net.URI;
 import java.nio.file.Path;
+import java.time.Instant;
 import java.util.Collections;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
+import me.itzg.helpers.errors.RateLimitException;
 import me.itzg.helpers.github.model.Asset;
 import me.itzg.helpers.github.model.Release;
 import me.itzg.helpers.http.FailedRequestException;
@@ -34,9 +41,25 @@ public Mono<Path> downloadLatestAsset(String org, String repo, @Nullable Pattern
             .acceptContentTypes(Collections.singletonList("application/vnd.github+json"))
             .toObject(Release.class)
             .assemble()
-            .onErrorResume(throwable -> throwable instanceof FailedRequestException && ((FailedRequestException) throwable).getStatusCode() == 404,
-                throwable -> Mono.empty()
-            )
+            .onErrorResume(throwable -> {
+                if (throwable instanceof FailedRequestException) {
+                    final FailedRequestException fre = (FailedRequestException) throwable;
+                    if (fre.getStatusCode() == HttpResponseStatus.NOT_FOUND.code()) {
+                        return Mono.empty();
+                    }
+
+                    if ((fre.getStatusCode() == FORBIDDEN.code() || fre.getStatusCode() == TOO_MANY_REQUESTS.code())) {
+                        final HttpHeaders headers = fre.getHeaders();
+                        final String resetTimeStr = headers.get("x-ratelimit-reset");
+                        if (resetTimeStr != null) {
+                            return Mono.error(new RateLimitException(Instant.ofEpochSecond(Long.parseLong(resetTimeStr)),
+                                "Rate-limit exceeded", fre
+                                ));
+                        }
+                    }
+                }
+                return Mono.error(throwable);
+            })
             .flatMap(release -> {
                 if (log.isDebugEnabled()) {
                     log.debug("Assets in latest release '{}': {}",
diff --git a/src/main/java/me/itzg/helpers/http/FailedRequestException.java b/src/main/java/me/itzg/helpers/http/FailedRequestException.java
@@ -1,5 +1,6 @@
 package me.itzg.helpers.http;
 
+import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import java.net.URI;
 import lombok.Getter;
@@ -11,31 +12,34 @@ public class FailedRequestException extends RuntimeException {
     private final URI uri;
     private final int statusCode;
     private final String body;
+    private final HttpHeaders headers;
 
     /**
      * Reactor Netty flavor
      */
-    public FailedRequestException(HttpResponseStatus status, URI uri, String body, String msg) {
+    public FailedRequestException(HttpResponseStatus status, URI uri, String body, String msg, HttpHeaders headers) {
         super(
             String.format("HTTP request of %s failed with %s: %s", uri, status, msg)
         );
         this.uri = uri;
         this.statusCode = status.code();
         this.body = body;
+        this.headers = headers;
     }
 
     @SuppressWarnings("unused")
     public static boolean isNotFound(Throwable throwable) {
         return isStatus(throwable, HttpResponseStatus.NOT_FOUND);
     }
 
-    public static boolean isBadRequest(Throwable throwable) {
-        return isStatus(throwable, HttpResponseStatus.BAD_REQUEST);
-    }
-
-    private static boolean isStatus(Throwable throwable, HttpResponseStatus status) {
+    public static boolean isStatus(Throwable throwable, HttpResponseStatus... statuses) {
         if (throwable instanceof FailedRequestException) {
-            return ((FailedRequestException) throwable).getStatusCode() == status.code();
+            final int actualStatus = ((FailedRequestException) throwable).getStatusCode();
+            for (final HttpResponseStatus status : statuses) {
+                if (status.code() == actualStatus) {
+                    return true;
+                }
+            }
         }
         return false;
     }
diff --git a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java
@@ -192,7 +192,7 @@ private static List<String> applyHeaderRedactions(HttpHeaders headers) {
     protected  <R> Mono<R> failedRequestMono(HttpClientResponse resp, ByteBufMono bodyMono, String description) {
         return (bodyMono != null ? bodyMono.asString() : Mono.just(""))
             .defaultIfEmpty("")
-            .flatMap(body -> Mono.error(new FailedRequestException(resp.status(), uri(), body, description)));
+            .flatMap(body -> Mono.error(new FailedRequestException(resp.status(), uri(), body, description, resp.responseHeaders())));
     }
 
     protected static boolean notSuccess(HttpClientResponse resp) {
diff --git a/src/test/java/me/itzg/helpers/LatchingExecutionExceptionHandler.java b/src/test/java/me/itzg/helpers/LatchingExecutionExceptionHandler.java
diff --git a/src/test/java/me/itzg/helpers/github/DownloadLatestAssetCommandTest.java b/src/test/java/me/itzg/helpers/github/DownloadLatestAssetCommandTest.java

Original file line number	Diff line number	Diff line change
`@@ -192,7 +192,7 @@ private static List<String> applyHeaderRedactions(HttpHeaders headers) {`
`192`	`192`	`protected <R> Mono<R> failedRequestMono(HttpClientResponse resp, ByteBufMono bodyMono, String description) {`
`193`	`193`	`return (bodyMono != null ? bodyMono.asString() : Mono.just(""))`
`194`	`194`	`.defaultIfEmpty("")`
`195`		`- .flatMap(body -> Mono.error(new FailedRequestException(resp.status(), uri(), body, description)));`
	`195`	`+ .flatMap(body -> Mono.error(new FailedRequestException(resp.status(), uri(), body, description, resp.responseHeaders())));`
`196`	`196`	`}`
`197`	`197`
`198`	`198`	`protected static boolean notSuccess(HttpClientResponse resp) {`