cf: remove failed file downloads (#626)

Also
* Switch default to http 1.1
* Include retry number in CF file

Author: itzg

src/main/java/me/itzg/helpers/curseforge/CurseForgeInstaller.java

@@ -50,6 +50,7 @@
 import me.itzg.helpers.errors.InvalidParameterException;
 import me.itzg.helpers.fabric.FabricLauncherInstaller;
 import me.itzg.helpers.files.Manifests;
+import me.itzg.helpers.files.ReactiveFileUtils;
 import me.itzg.helpers.files.ResultsFileWriter;
 import me.itzg.helpers.forge.ForgeInstaller;
 import me.itzg.helpers.forge.ForgeInstallerResolver;
@@ -763,10 +764,15 @@ private Mono<PathWithInfo> downloadAndPostProcess(
     private  Mono<DownloadOrResolveResult> buildRetryableDownload(InstallContext context,
         CurseForgeMod modInfo, CurseForgeFile cfFile, boolean isWorld, Path outputSubdir
     ) {
+        final Path outputFile = resolveOutputFile(outputSubdir, cfFile);
+
         // use defer so that the download mono is rebuilt on each retry
         return Mono.defer(() ->
                 downloadOrResolveFile(context, modInfo, isWorld, outputSubdir, cfFile)
                     .checkpoint()
+                    .onErrorResume(throwable ->
+                        ReactiveFileUtils.removeFailedDownload(throwable, outputFile)
+                    )
             )
             // retry the deferred part above if one of the expected failure cases
             .retryWhen(
@@ -778,8 +784,10 @@ private  Mono<DownloadOrResolveResult> buildRetryableDownload(InstallContext con
                             throwable instanceof ChannelException
                     )
                     .doBeforeRetry(retrySignal ->
-                        log.warn("Retrying to download {} @ {}:{}",
-                            cfFile.getFileName(), modInfo.getName(), cfFile.getDisplayName()
+                        log.warn("Retry #{} download of {} @ {}:{} due to {}",
+                            retrySignal.totalRetries() + 1,
+                            cfFile.getFileName(), modInfo.getName(), cfFile.getDisplayName(),
+                            retrySignal.failure().getClass().getSimpleName()
                         )
                     )
             );
@@ -799,7 +807,7 @@ static class DownloadOrResolveResult {
     private Mono<DownloadOrResolveResult> downloadOrResolveFile(InstallContext context, CurseForgeMod modInfo,
         boolean isWorld, Path outputSubdir, CurseForgeFile cfFile
     ) {
-        final Path outputFile = outputSubdir.resolve(cfFile.getFileName());
+        final Path outputFile = resolveOutputFile(outputSubdir, cfFile);
 
         // Will try to locate an existing file by alternate names that browser might create,
         // but only for non-world files of the modpack
@@ -829,6 +837,10 @@ private Mono<DownloadOrResolveResult> downloadOrResolveFile(InstallContext conte
         }
     }
 
+    private static @NotNull Path resolveOutputFile(Path outputSubdir, CurseForgeFile cfFile) {
+        return outputSubdir.resolve(cfFile.getFileName());
+    }
+
     /**
      * @return Mono.error with {@link FileHashInvalidException} when not valid
      */

src/main/java/me/itzg/helpers/fabric/FabricLauncherInstaller.java

@@ -66,12 +66,14 @@ public void installUsingVersions(
 
                             fabricMetaClient.resolveInstallerVersion(installerVersion)
                                 .doOnNext(v -> log.debug("Resolved installer version {} from {}", v, installerVersion))
-                                .flatMap(resolvedInstallerVersion -> downloadResolvedLauncher(
-                                    fabricMetaClient,
-                                    resolvedMinecraftVersion,
-                                    resolvedLoaderVersion,
-                                    resolvedInstallerVersion
-                                ))
+                                .flatMap(resolvedInstallerVersion ->
+                                    downloadResolvedLauncher(
+                                        fabricMetaClient,
+                                        resolvedMinecraftVersion,
+                                        resolvedLoaderVersion,
+                                        resolvedInstallerVersion
+                                    )
+                                )
                                 .checkpoint("downloadResolvedLauncher")
                         )
                 )

src/main/java/me/itzg/helpers/files/ReactiveFileUtils.java

@@ -1,6 +1,7 @@
 package me.itzg.helpers.files;
 
 import io.netty.buffer.ByteBuf;
+import java.io.IOException;
 import java.nio.channels.FileChannel;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -84,4 +85,26 @@ public static Mono<Long> writeByteBufFluxToFile(ByteBufFlux byteBufFlux, Path fi
             // Just expose the total bytes read from network
             .map(Tuple2::getT2);
     }
+
+    /**
+     * Used with {@link reactor.core.publisher.Mono#onErrorResume(java.util.function.Function)}
+     * @param throwable the throwable to pass to the returned mono
+     * @param outputFile the file to remove, if present
+     * @return an error mono with the given throwable as cause
+     * @param <T> the type of the surrounding reactive chain
+     */
+    public static <T> Mono<T> removeFailedDownload(Throwable throwable, Path outputFile) {
+        return Mono.<T>create(sink -> {
+                try {
+                    log.trace("Removing failed download of {}", outputFile);
+                    if (Files.deleteIfExists(outputFile)) {
+                        log.debug("Removed failed download of {}", outputFile);
+                    }
+                } catch (IOException e) {
+                    log.error("Unable to remove failed download of {}", outputFile, e);
+                }
+                sink.error(throwable);
+            })
+            .subscribeOn(Schedulers.boundedElastic());
+    }
 }

src/main/java/me/itzg/helpers/http/SharedFetchArgs.java

@@ -48,7 +48,7 @@ public void setPendingAcquireTimeout(Duration timeout) {
         optionsBuilder.pendingAcquireTimeout(timeout);
     }
 
-    @Option(names = "--use-http2", defaultValue = "${env:FETCH_USE_HTTP2:-true}",
+    @Option(names = "--use-http2", defaultValue = "${env:FETCH_USE_HTTP2:-false}",
         description = "Whether to use HTTP/2. Default: ${DEFAULT-VALUE}"
     )
     public void setUseHttp2(boolean useHttp2) {