Address CodeRabbit review feedback

- Let Cancel, Escape, and the backdrop abort an in-flight COG export. The
  retry ladder could otherwise hold the dialog for the better part of two
  minutes with no way out. An AbortSignal is threaded through downloadCog
  and fetchExport, where it is merged with the per-attempt deadline, and a
  cancel is no longer reported as a failure.
- Stop the retry ladder on the longer side rather than on both sides. The
  previous guard was per-axis-independent, so a 2048x512 request kept
  halving while only its width cleared the floor. Gating on the shorter
  side instead would cut the ladder short for an ordinary 16:9 view, which
  is when the retry is most useful, so the decision now lives in a tested
  nextExportSize helper.
- Report the size actually delivered on success, since a retry can step it
  below what the dialog previewed.

Author: giswqs

apps/geolibre-desktop/src/components/layout/TopToolbar.tsx

@@ -319,7 +319,8 @@ export function TopToolbar({
       cogDownload: t("earthdataGis.cogDownload"),
       cogDownloading: t("earthdataGis.cogDownloading"),
       cogRetrying: (width, height) => t("earthdataGis.cogRetrying", { width, height }),
-      cogDone: t("earthdataGis.cogDone"),
+      cogConverting: t("earthdataGis.cogConverting"),
+      cogDone: (width, height) => t("earthdataGis.cogDone", { width, height }),
       cogFailed: (message) => t("earthdataGis.cogFailed", { message }),
       cogUnavailable: t("earthdataGis.cogUnavailable"),
       cancel: t("earthdataGis.cancel"),

apps/geolibre-desktop/src/i18n/locales/en.json

@@ -2743,7 +2743,8 @@
     "cogDownload": "Download",
     "cogDownloading": "Exporting and converting…",
     "cogRetrying": "The service refused that size; retrying at {{width}} x {{height}} px…",
-    "cogDone": "Saved the COG.",
+    "cogConverting": "Converting to a COG…",
+    "cogDone": "Saved the COG at {{width}} x {{height}} px.",
     "cogFailed": "Could not download the COG: {{message}}",
     "cogUnavailable": "Downloading requires the desktop app or a browser save dialog.",
     "cancel": "Cancel",

packages/plugins/src/plugins/earthdata-gis-api.ts

@@ -376,6 +376,29 @@ export function exportImageSize(
   };
 }
 
+/**
+ * Halves an export size for the next retry, or reports that the ladder is
+ * exhausted.
+ *
+ * The floor is compared against the **longer** side. Testing both sides made it
+ * per-axis-independent — a 2048x512 request would keep halving as long as the
+ * width alone cleared the floor — while testing the shorter side would stop the
+ * ladder after one step for an ordinary 16:9 view, which is exactly the case
+ * the retry exists to rescue.
+ *
+ * @param size - The size that just failed
+ * @param minPixels - Smallest longer-side export worth attempting
+ * @returns The next size to try, or null when nothing smaller is worth asking for
+ */
+export function nextExportSize(
+  size: { width: number; height: number },
+  minPixels: number,
+): { width: number; height: number } | null {
+  const next = { width: Math.round(size.width / 2), height: Math.round(size.height / 2) };
+  if (Math.max(next.width, next.height) < minPixels) return null;
+  return next;
+}
+
 /**
  * Builds a concrete (non-templated) export URL for one area, used for the
  * GeoTIFF download rather than for map tiles.

packages/plugins/src/plugins/maplibre-earthdata-gis.ts

@@ -32,6 +32,7 @@ import {
   fetchMinVisibleZoom,
   fetchWebMapLayers,
   HTTP_URL_RE,
+  nextExportSize,
   searchEarthdataGis,
   type WebMapLayer,
   webMapLayerAsItem,
@@ -135,7 +136,8 @@ export interface EarthdataGisLabels {
   cogDownload: string;
   cogDownloading: string;
   cogRetrying: (width: number, height: number) => string;
-  cogDone: string;
+  cogConverting: string;
+  cogDone: (width: number, height: number) => string;
   cogFailed: (message: string) => string;
   cogUnavailable: string;
   cancel: string;
@@ -212,7 +214,8 @@ export const DEFAULT_EARTHDATA_GIS_LABELS: EarthdataGisLabels = {
   cogDownloading: "Exporting and converting…",
   cogRetrying: (width, height) =>
     `The service refused that size; retrying at ${width} x ${height} px…`,
-  cogDone: "Saved the COG.",
+  cogConverting: "Converting to a COG…",
+  cogDone: (width, height) => `Saved the COG at ${width} x ${height} px.`,
   cogFailed: (message) => `Could not download the COG: ${message}`,
   cogUnavailable: "Downloading requires the desktop app or a browser save dialog.",
   cancel: "Cancel",
@@ -696,6 +699,10 @@ function openCogModal(item: EarthdataGisItem): void {
   let limits: ExportLimits = { maxWidth: 4096, maxHeight: 4096 };
   let serviceExtent3857: [number, number, number, number] | null = null;
   let running = false;
+  // Set while an export is in flight so Cancel / Escape / the backdrop can stop
+  // it: the retry ladder can otherwise hold the dialog for the better part of
+  // two minutes with no way out.
+  let inflight: AbortController | null = null;
 
   const refresh = (): void => {
     // A service with no published extent can only be exported over the map view.
@@ -727,20 +734,20 @@ function openCogModal(item: EarthdataGisItem): void {
   }
 
   const close = (): void => {
+    inflight?.abort();
+    inflight = null;
     overlay.remove();
     document.removeEventListener("keydown", onKey);
     previouslyFocused?.focus?.();
     if (closeDetailsDialog === close) closeDetailsDialog = null;
   };
   const onKey = (event: KeyboardEvent): void => {
-    if (event.key === "Escape" && !running) close();
+    if (event.key === "Escape") close();
   };
   overlay.addEventListener("click", (event) => {
-    if (event.target === overlay && !running) close();
-  });
-  cancelButton.addEventListener("click", () => {
-    if (!running) close();
+    if (event.target === overlay) close();
   });
+  cancelButton.addEventListener("click", close);
 
   downloadButton.addEventListener("click", () => {
     if (running) return;
@@ -755,12 +762,23 @@ function openCogModal(item: EarthdataGisItem): void {
     downloadButton.disabled = true;
     status.style.color = "hsl(var(--muted-foreground))";
     status.textContent = labels.cogDownloading;
-    void downloadCog(item, bounds, limits, (message) => {
-      status.textContent = message;
-    })
-      .then((saved) => {
-        if (saved) {
-          status.textContent = labels.cogDone;
+    const controller = new AbortController();
+    inflight = controller;
+    void downloadCog(
+      item,
+      bounds,
+      limits,
+      (message) => {
+        status.textContent = message;
+      },
+      controller.signal,
+    )
+      .then((result) => {
+        if (controller.signal.aborted) return;
+        if (result.saved && result.size) {
+          // Report the size actually delivered: the retry ladder may have
+          // stepped it below what the dialog previewed.
+          status.textContent = labels.cogDone(result.size.width, result.size.height);
           close();
         } else {
           // The user dismissed the save dialog; leave the modal open so the
@@ -769,14 +787,16 @@ function openCogModal(item: EarthdataGisItem): void {
         }
       })
       .catch((error: unknown) => {
+        if (controller.signal.aborted) return;
         status.style.color = "hsl(var(--destructive))";
         status.textContent = labels.cogFailed(
           error instanceof Error ? error.message : String(error),
         );
       })
       .finally(() => {
         running = false;
-        refresh();
+        if (inflight === controller) inflight = null;
+        if (overlay.isConnected) refresh();
       });
   });
 
@@ -794,20 +814,28 @@ function openCogModal(item: EarthdataGisItem): void {
   });
 }
 
+/** What a COG download produced: whether a file was written, and at what size. */
+interface CogDownloadResult {
+  saved: boolean;
+  /** The size actually exported, which the retry ladder may have reduced. */
+  size: { width: number; height: number } | null;
+}
+
 /**
  * Exports the area as a GeoTIFF and hands it to the host to be re-encoded as a
  * COG and saved.
  *
- * @returns True when a file was written, false when the save was cancelled
+ * @returns Whether a file was written, and the size it was exported at
  * @throws When the export request fails or returns something other than a TIFF
  */
 async function downloadCog(
   item: EarthdataGisItem,
   bbox3857: [number, number, number, number],
   limits: ExportLimits,
   onProgress?: (message: string) => void,
-): Promise<boolean> {
-  if (!cogSaver) return false;
+  signal?: AbortSignal,
+): Promise<CogDownloadResult> {
+  if (!cogSaver) return { saved: false, size: null };
   let size = exportImageSize(bbox3857, cappedLimits(limits));
   let lastError: Error | null = null;
 
@@ -816,16 +844,21 @@ async function downloadCog(
   // at 4096px and answers 503 at 4977px. Step the request down on failure so a
   // service that cannot manage the first size still yields a file.
   for (let attempt = 0; attempt < EXPORT_ATTEMPTS; attempt += 1) {
+    if (signal?.aborted) return { saved: false, size: null };
     const url = buildExportDownloadUrl(item, bbox3857, size, "tiff");
-    if (!url) return false;
+    if (!url) return { saved: false, size: null };
     if (attempt > 0) onProgress?.(labels.cogRetrying(size.width, size.height));
     try {
-      const bytes = await fetchExport(url);
-      return cogSaver(bytes, exportFileName(item.title, "tif"));
+      const bytes = await fetchExport(url, signal);
+      onProgress?.(labels.cogConverting);
+      const saved = await cogSaver(bytes, exportFileName(item.title, "tif"));
+      return { saved, size };
     } catch (error) {
+      // A cancel is the user's decision, not a failure to retry or report.
+      if (signal?.aborted) return { saved: false, size: null };
       lastError = error instanceof Error ? error : new Error(String(error));
-      const next = { width: Math.round(size.width / 2), height: Math.round(size.height / 2) };
-      if (next.width < EXPORT_MIN_PIXELS && next.height < EXPORT_MIN_PIXELS) break;
+      const next = nextExportSize(size, EXPORT_MIN_PIXELS);
+      if (!next) break;
       size = next;
     }
   }
@@ -840,19 +873,26 @@ async function downloadCog(
  * @throws With the service's own message when it reports an error, or a
  *   timeout/status message otherwise
  */
-async function fetchExport(url: string): Promise<Uint8Array> {
+async function fetchExport(url: string, signal?: AbortSignal): Promise<Uint8Array> {
+  // One controller fed by both the per-attempt deadline and the caller's cancel,
+  // so either can stop the request. (AbortSignal.any would do this, but it is
+  // newer than the browsers this app still targets.)
   const controller = new AbortController();
   const timer = setTimeout(() => controller.abort(), EXPORT_ATTEMPT_TIMEOUT_MS);
+  const onCancel = (): void => controller.abort();
+  signal?.addEventListener("abort", onCancel);
   let response: Response;
   try {
     response = await fetch(url, { signal: controller.signal });
   } catch (error) {
     if (error instanceof DOMException && error.name === "AbortError") {
+      if (signal?.aborted) throw error;
       throw new Error("the service did not respond in time at this size");
     }
     throw error;
   } finally {
     clearTimeout(timer);
+    signal?.removeEventListener("abort", onCancel);
   }
   if (!response.ok) throw new Error(`the service returned ${response.status} at this size`);
 

tests/earthdata-gis-api.test.ts

@@ -19,6 +19,7 @@ import {
   exportFileName,
   exportImageSize,
   fetchExportLimits,
+  nextExportSize,
   parseMaxPixelSize,
   parseSearchResponse,
   parseWebMapLayers,
@@ -559,6 +560,35 @@ describe("earthdata gis api", () => {
       });
     });
 
+    describe("nextExportSize", () => {
+      it("halves a square export until the floor", () => {
+        assert.deepEqual(nextExportSize({ width: 2048, height: 2048 }, 512), {
+          width: 1024,
+          height: 1024,
+        });
+        assert.equal(nextExportSize({ width: 512, height: 512 }, 512), null);
+      });
+
+      it("keeps stepping a 16:9 view down, where the retry is most useful", () => {
+        // Gating on the shorter side would stop after one step here.
+        assert.deepEqual(nextExportSize({ width: 2048, height: 1152 }, 512), {
+          width: 1024,
+          height: 576,
+        });
+        assert.deepEqual(nextExportSize({ width: 1024, height: 576 }, 512), {
+          width: 512,
+          height: 288,
+        });
+      });
+
+      it("stops once the longer side falls under the floor", () => {
+        // The previous guard tested both axes, so a thin export kept halving as
+        // long as its width alone cleared the floor.
+        assert.equal(nextExportSize({ width: 512, height: 128 }, 512), null);
+        assert.equal(nextExportSize({ width: 900, height: 100 }, 512), null);
+      });
+    });
+
     describe("buildExportDownloadUrl", () => {
       it("requests a concrete bbox and a TIFF, not a tile template", () => {
         const url = new URL(