From 2b685b5fa01d3233916d2910022d7226dcbdacb2 Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Fri, 17 Mar 2023 19:17:47 -0500 Subject: [PATCH 1/2] cf: report missing manifest as invalid parameter --- .../curseforge/CurseForgeInstaller.java | 33 +++++++++++++++++-- .../curseforge/model/CurseForgeResponse.java | 10 ++++++ .../helpers/http/FailedRequestException.java | 8 +++-- .../itzg/helpers/http/FetchBuilderBase.java | 6 ++-- .../itzg/helpers/http/ObjectFetchBuilder.java | 2 +- .../http/OutputToDirectoryFetchBuilder.java | 13 ++++---- .../http/SpecificFileFetchBuilder.java | 2 +- 7 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 src/main/java/me/itzg/helpers/curseforge/model/CurseForgeResponse.java diff --git a/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java b/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java index 34462cad..fe2109f7 100644 --- a/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java +++ b/src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java @@ -4,6 +4,8 @@ import static java.util.Objects.requireNonNull; import static me.itzg.helpers.curseforge.MoreCollections.safeStreamFrom; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import java.io.IOException; import java.net.URI; import java.nio.file.Files; @@ -32,6 +34,7 @@ import me.itzg.helpers.curseforge.model.Category; import me.itzg.helpers.curseforge.model.CurseForgeFile; import me.itzg.helpers.curseforge.model.CurseForgeMod; +import me.itzg.helpers.curseforge.model.CurseForgeResponse; import me.itzg.helpers.curseforge.model.GetCategoriesResponse; import me.itzg.helpers.curseforge.model.GetModFileResponse; import me.itzg.helpers.curseforge.model.GetModFilesResponse; @@ -41,10 +44,12 @@ import me.itzg.helpers.curseforge.model.ModLoader; import me.itzg.helpers.curseforge.model.ModsSearchResponse; import me.itzg.helpers.errors.GenericException; +import me.itzg.helpers.errors.InvalidParameterException; import me.itzg.helpers.fabric.FabricLauncherInstaller; import me.itzg.helpers.files.Manifests; import me.itzg.helpers.files.ResultsFileWriter; import me.itzg.helpers.forge.ForgeInstaller; +import me.itzg.helpers.http.FailedRequestException; import me.itzg.helpers.http.Fetch; import me.itzg.helpers.http.SharedFetch; import me.itzg.helpers.http.SharedFetch.Options; @@ -369,12 +374,12 @@ private ExcludeIncludeIds resolveExcludeIncludes( SharedFetch preparedFetch, UriBuilder uriBuilder, CategoryInfo categoryInfo, String modpackSlug ) { - log.debug("Reconciling exclude/includes from given {}", excludeIncludes); - if (excludeIncludes == null) { return new ExcludeIncludeIds(emptySet(), emptySet()); } + log.debug("Reconciling exclude/includes from given {}", excludeIncludes); + final ExcludeIncludes specific = excludeIncludes.getModpacks() != null ? excludeIncludes.getModpacks().get(modpackSlug) : null; @@ -679,9 +684,29 @@ private static Mono getModFileInfo(SharedFetch preparedFetch, Ur ) .toObject(GetModFileResponse.class) .assemble() + .onErrorMap(FailedRequestException.class::isInstance, e -> { + final FailedRequestException fre = (FailedRequestException) e; + if (fre.getStatusCode() == 400) { + if (isNotFoundResponse(fre.getBody())) { + return new InvalidParameterException("Requested file not found for modpack", e); + } + } + return e; + }) .map(GetModFileResponse::getData); } + private static boolean isNotFoundResponse(String body) { + try { + final CurseForgeResponse resp = ObjectMappers.defaultMapper().readValue( + body, new TypeReference>() {} + ); + return resp.getError().startsWith("Error: 404"); + } catch (JsonProcessingException e) { + throw new GenericException("Unable to parse error response", e); + } + } + private static Mono getModInfo( SharedFetch preparedFetch, UriBuilder uriBuilder, int projectID @@ -733,7 +758,9 @@ private MinecraftModpackManifest extractModpackManifest(Path modpackZip) throws } } - throw new GenericException("Modpack is missing manifest.json. Make sure to reference a non-server file."); + throw new InvalidParameterException( + "Modpack file is missing a manifest. Make sure to reference a non-server file." + ); } } diff --git a/src/main/java/me/itzg/helpers/curseforge/model/CurseForgeResponse.java b/src/main/java/me/itzg/helpers/curseforge/model/CurseForgeResponse.java new file mode 100644 index 00000000..ab514c85 --- /dev/null +++ b/src/main/java/me/itzg/helpers/curseforge/model/CurseForgeResponse.java @@ -0,0 +1,10 @@ +package me.itzg.helpers.curseforge.model; + +import lombok.Data; + +@Data +public class CurseForgeResponse { + private boolean ok; + private String error; + private T data; +} diff --git a/src/main/java/me/itzg/helpers/http/FailedRequestException.java b/src/main/java/me/itzg/helpers/http/FailedRequestException.java index d44b65fc..e04ac84b 100644 --- a/src/main/java/me/itzg/helpers/http/FailedRequestException.java +++ b/src/main/java/me/itzg/helpers/http/FailedRequestException.java @@ -3,23 +3,25 @@ import io.netty.handler.codec.http.HttpResponseStatus; import java.net.URI; import lombok.Getter; +import lombok.ToString; +@Getter @ToString public class FailedRequestException extends RuntimeException { - @Getter private final URI uri; - @Getter private final int statusCode; + private final String body; /** * Reactor Netty flavor */ - public FailedRequestException(HttpResponseStatus status, URI uri, String msg) { + public FailedRequestException(HttpResponseStatus status, URI uri, String body, String msg) { super( String.format("HTTP request of %s failed with %s: %s", uri, status, msg) ); this.uri = uri; this.statusCode = status.code(); + this.body = body; } @SuppressWarnings("unused") diff --git a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java index d9d0ff73..d7f51094 100644 --- a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java +++ b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java @@ -21,6 +21,7 @@ import me.itzg.helpers.json.ObjectMappers; import org.slf4j.Logger; import reactor.core.publisher.Mono; +import reactor.netty.ByteBufMono; import reactor.netty.Connection; import reactor.netty.http.client.HttpClient; import reactor.netty.http.client.HttpClientRequest; @@ -144,8 +145,9 @@ protected R useReactiveClient(ReactiveClientUser user) { ); } - protected Mono failedRequestMono(HttpClientResponse resp, String description) { - return Mono.error(new FailedRequestException(resp.status(), uri(), description)); + protected Mono failedRequestMono(HttpClientResponse resp, ByteBufMono bodyMono, String description) { + return (bodyMono != null ? bodyMono.asString() : Mono.just("")) + .flatMap(body -> Mono.error(new FailedRequestException(resp.status(), uri(), body, description))); } protected static boolean notSuccess(HttpClientResponse resp) { diff --git a/src/main/java/me/itzg/helpers/http/ObjectFetchBuilder.java b/src/main/java/me/itzg/helpers/http/ObjectFetchBuilder.java index 5033bda2..d03c7750 100644 --- a/src/main/java/me/itzg/helpers/http/ObjectFetchBuilder.java +++ b/src/main/java/me/itzg/helpers/http/ObjectFetchBuilder.java @@ -56,7 +56,7 @@ private Mono assembleCommon() { private Mono handleResponse(HttpClientResponse resp, ByteBufMono bodyMono) { if (notSuccess(resp)) { - return failedRequestMono(resp, "Fetching object content"); + return failedRequestMono(resp, bodyMono, "Fetching object content"); } if (notExpectedContentType(resp)) { return failedContentTypeMono(resp); diff --git a/src/main/java/me/itzg/helpers/http/OutputToDirectoryFetchBuilder.java b/src/main/java/me/itzg/helpers/http/OutputToDirectoryFetchBuilder.java index 9ef79cd6..740ad1ef 100644 --- a/src/main/java/me/itzg/helpers/http/OutputToDirectoryFetchBuilder.java +++ b/src/main/java/me/itzg/helpers/http/OutputToDirectoryFetchBuilder.java @@ -60,11 +60,10 @@ public Mono assemble() { .doOnRequest(debugLogRequest(log, "file head fetch")) .head() .uri(uri()) - .response() - .flatMap(resp -> - notSuccess(resp) ? failedRequestMono(resp, "Extracting filename") + .responseSingle((resp, bodyMono) -> + notSuccess(resp) ? failedRequestMono(resp, bodyMono, "Extracting filename") : Mono.just(outputDirectory.resolve(extractFilename(resp))) - ) + ) .flatMap(outputFile -> assembleFileDownload(client, outputFile) ) @@ -84,12 +83,12 @@ private Mono assembleFileDownload(HttpClient client, Path outputFile) { .doOnRequest(debugLogRequest(log, "file fetch")) .get() .uri(uri()) - .responseSingle((resp, byteBufMono) -> { + .responseSingle((resp, bodyMono) -> { if (notSuccess(resp)) { - return failedRequestMono(resp, "Downloading file"); + return failedRequestMono(resp, bodyMono, "Downloading file"); } - return byteBufMono.asInputStream() + return bodyMono.asInputStream() .publishOn(Schedulers.boundedElastic()) .flatMap(inputStream -> { try { diff --git a/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java b/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java index 0c8ae9f4..1359af90 100644 --- a/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java +++ b/src/main/java/me/itzg/helpers/http/SpecificFileFetchBuilder.java @@ -106,7 +106,7 @@ public Mono assemble() { } if (notSuccess(resp)) { - return failedRequestMono(resp, "Trying to retrieve file"); + return failedRequestMono(resp, bodyMono, "Trying to retrieve file"); } if (notExpectedContentType(resp)) { From 300d7b346020c53d2255544a26ceabf6ed8ca858 Mon Sep 17 00:00:00 2001 From: Geoff Bourne Date: Fri, 17 Mar 2023 19:27:56 -0500 Subject: [PATCH 2/2] Fixed empty body handling during failed response --- src/main/java/me/itzg/helpers/http/FetchBuilderBase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java index d7f51094..71a486d6 100644 --- a/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java +++ b/src/main/java/me/itzg/helpers/http/FetchBuilderBase.java @@ -147,6 +147,7 @@ protected R useReactiveClient(ReactiveClientUser user) { protected Mono 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))); }