Retries

Closes #95

TODO.md

@@ -8,8 +8,6 @@ P2
 
 Planned for later.
 
-* Soft errors and retry logic
-  * Big caveat: When the server still responds, like 50x errors which would be recoverable, we actually have no way of knowing it did in respond in such a way. See P4 - Handle Errors remarks.
 * Inter-addon API (basic)
   * Add downloads
 * vtable perf: cache column widths
@@ -44,8 +42,6 @@ Stuff that probably cannot be implemented due to WeberEension limitations.
   * Firefox helpfully keeps different lists of downloads. One for newly added downloads, and other ones for "previous" downloads. Turns out the WebExtension API only ever queries the "new" list.
 * Segmented downloads
   * Cannot be done with WebExtensions - downloads API has no support and manually downloading, storing in temporary add-on storage and reassmbling the downloaded parts later is not only efficient but does not reliabliy work due to storage limitations.
-* Handle errors, 404 and such
-  * The Firefox download manager is too stupid and webRequest does not see Downloads, so cannot be done right now.
 * Conflicts: ask when a file exists
   * Not supported by Firefox
 * Speed limiter

_locales/en/messages.json

@@ -617,6 +617,14 @@
     "description": "Preferences/General",
     "message": "Remove missing downloads after a restart"
   },
+  "pref_retries": {
+    "description": "pref text",
+    "message": "Number of retries of downloads on temporary errors"
+  },
+  "pref_retry_time": {
+    "description": "pref text",
+    "message": "Retry every (in minutes)"
+  },
   "pref_show_urls": {
     "description": "Preferences/General",
     "message": "Show URLs instead of Names"
@@ -955,6 +963,20 @@
     "description": "Action for resuming a download",
     "message": "Resume"
   },
+  "retrying": {
+    "description": "Status text",
+    "message": "Retrying"
+  },
+  "retrying_error": {
+    "description": "status text",
+    "message": "Retrying - $ERROR$",
+    "placeholders": {
+      "error": {
+        "content": "$1",
+        "example": "Server Error"
+      }
+    }
+  },
   "running": {
     "description": "Status text",
     "message": "Running"

data/prefs.json

@@ -15,6 +15,8 @@
 	"tooltip": true,
 	"show-urls": false,
 	"remove-missing-on-init": false,
+	"retries": 5,
+	"retry-time": 10,
 	"limits": [
 		{
 			"domain": "*",

lib/db.ts

@@ -2,6 +2,9 @@
 
 // eslint-disable-next-line no-unused-vars
 import { BaseItem } from "./item";
+// eslint-disable-next-line no-unused-vars
+import { Download } from "./manager/download";
+import { RUNNING, QUEUED, RETRYING } from "./manager/state";
 
 // License: MIT
 
@@ -69,7 +72,7 @@ export const DB = new class DB {
     return await new Promise(this.getAllInternal);
   }
 
-  saveItemsInternal(items: any[], resolve: Function, reject: Function) {
+  saveItemsInternal(items: Download[], resolve: Function, reject: Function) {
     if (!items || !items.length || !this.db) {
       resolve();
       return;
@@ -83,9 +86,13 @@ export const DB = new class DB {
         if (item.private) {
           continue;
         }
-        const req = store.put(item.toJSON());
+        const json = item.toJSON();
+        if (item.state === RUNNING || item.state === RETRYING) {
+          json.state = QUEUED;
+        }
+        const req = store.put(json);
         if (!("dbId" in item) || item.dbId < 0) {
-          req.onsuccess = () => item.dbId = req.result;
+          req.onsuccess = () => item.dbId = req.result as number;
         }
       }
     }
@@ -94,7 +101,7 @@ export const DB = new class DB {
     }
   }
 
-  async saveItems(items: any[]) {
+  async saveItems(items: Download[]) {
     await this.init();
     return await new Promise(this.saveItemsInternal.bind(this, items));
   }

lib/manager/basedownload.ts

@@ -48,7 +48,9 @@ const DEFAULTS = {
   written: 0,
   manId: 0,
   mime: "",
-  prerolled: false
+  prerolled: false,
+  retries: 0,
+  deadline: 0
 };
 
 let sessionId = 0;
@@ -105,6 +107,8 @@ export class BaseDownload {
 
   public prerolled: boolean;
 
+  public retries: number;
+
   constructor(options: any) {
     Object.assign(this, DEFAULTS);
     this.assign(options);
@@ -113,6 +117,7 @@ export class BaseDownload {
     }
     this.sessionId = ++sessionId;
     this.renamer = new Renamer(this);
+    this.retries = 0;
   }
 
   assign(options: any) {
@@ -182,6 +187,7 @@ export class BaseDownload {
     rv.currentName = this.browserName || rv.destName || rv.finalName;
     rv.error = this.error;
     rv.ext = this.renamer.p_ext;
+    rv.retries = this.retries;
     return rv;
   }
 }

lib/manager/download.ts

@@ -3,7 +3,7 @@
 
 // eslint-disable-next-line no-unused-vars
 import { CHROME, downloads, DownloadOptions } from "../browser";
-import { Prefs } from "../prefs";
+import { Prefs, PrefWatcher } from "../prefs";
 import { PromiseSerializer } from "../pserializer";
 import { filterInSitu, parsePath } from "../util";
 import { BaseDownload } from "./basedownload";
@@ -19,10 +19,27 @@ import {
   PAUSABLE,
   PAUSED,
   QUEUED,
-  RUNNING
+  RUNNING,
+  RETRYING
 } from "./state";
 import { Preroller } from "./preroller";
 
+function isRecoverable(error: string) {
+  switch (error) {
+  case "CRASH":
+    return true;
+
+  case "SERVER_FAILED":
+    return true;
+
+  default:
+    return error.startsWith("NETWORK_");
+  }
+}
+
+const RETRIES = new PrefWatcher("retries", 5);
+const RETRY_TIME = new PrefWatcher("retry-time", 5);
+
 export class Download extends BaseDownload {
   public manager: Manager;
 
@@ -34,6 +51,10 @@ export class Download extends BaseDownload {
 
   public error: string;
 
+  public dbId: number;
+
+  public deadline: number;
+
   constructor(manager: Manager, options: any) {
     super(options);
     this.manager = manager;
@@ -86,6 +107,7 @@ export class Download extends BaseDownload {
         return;
       }
       catch (ex) {
+        console.error("cannot resume", ex);
         this.manager.removeManId(this.manId);
         this.removeFromBrowser();
       }
@@ -182,8 +204,7 @@ export class Download extends BaseDownload {
         this.serverName = res.name;
       }
       if (res.error) {
-        this.cancel();
+        this.cancelAccordingToError(res.error);
-        this.error = res.error;
       }
     }
     catch (ex) {
@@ -209,20 +230,32 @@ export class Download extends BaseDownload {
     }
   }
 
-  async pause() {
+  async pause(retry?: boolean) {
     if (!(PAUSABLE & this.state)) {
       return;
     }
+
+    if (!retry) {
+      this.retries = 0;
+      this.deadline = 0;
+    }
+    else {
+      // eslint-disable-next-line no-magic-numbers
+      this.deadline = Date.now() + RETRY_TIME.value * 60 * 1000;
+    }
+
     if (this.state === RUNNING && this.manId) {
       try {
         await downloads.pause(this.manId);
       }
       catch (ex) {
         console.error("pause", ex.toString(), ex);
+        this.cancel();
         return;
       }
     }
-    this.changeState(PAUSED);
+
+    this.changeState(retry ? RETRYING : PAUSED);
   }
 
   reset() {
@@ -230,6 +263,8 @@ export class Download extends BaseDownload {
     this.manId = 0;
     this.written = this.totalSize = 0;
     this.mime = this.serverName = this.browserName = "";
+    this.retries = 0;
+    this.deadline = 0;
   }
 
   async removeFromBrowser() {
@@ -262,6 +297,17 @@ export class Download extends BaseDownload {
     this.changeState(CANCELED);
   }
 
+  async cancelAccordingToError(error: string) {
+    if (!isRecoverable(error) || ++this.retries > RETRIES.value) {
+      this.cancel();
+      this.error = error;
+      return;
+    }
+
+    await this.pause(true);
+    this.error = error;
+  }
+
   setMissing() {
     if (this.manId) {
       this.manager.removeManId(this.manId);
@@ -318,8 +364,7 @@ export class Download extends BaseDownload {
           this.changeState(PAUSED);
         }
         else if (error) {
-          this.cancel();
+          this.cancelAccordingToError(error);
-          this.error = error;
         }
         else {
           this.changeState(RUNNING);
@@ -330,6 +375,9 @@ export class Download extends BaseDownload {
         if (state.paused) {
           this.changeState(PAUSED);
         }
+        else if (error) {
+          this.cancelAccordingToError(error);
+        }
         else {
           this.cancel();
           this.error = error || "";

lib/manager/man.ts

@@ -4,11 +4,11 @@
 import { EventEmitter } from "../events";
 import { Notification } from "../notifications";
 import { DB } from "../db";
-import { QUEUED, CANCELED, RUNNING } from "./state";
+import { QUEUED, CANCELED, RUNNING, RETRYING } from "./state";
 // eslint-disable-next-line no-unused-vars
 import { Bus, Port } from "../bus";
 import { sort } from "../sorting";
-import { Prefs } from "../prefs";
+import { Prefs, PrefWatcher } from "../prefs";
 import { _ } from "../i18n";
 import { CoalescedUpdate, mapFilterInSitu, filterInSitu } from "../util";
 import { PromiseSerializer } from "../pserializer";
@@ -30,6 +30,9 @@ const setShelfEnabled = downloads.setShelfEnabled || function() {
   // ignored
 };
 
+const FINISH_NOTIFICATION = new PrefWatcher("finish-notification", true);
+const SOUNDS = new PrefWatcher("sounds", false);
+
 export class Manager extends EventEmitter {
   private items: Download[];
 
@@ -49,10 +52,14 @@ export class Manager extends EventEmitter {
 
   private readonly running: Set<Download>;
 
+  private readonly retrying: Set<Download>;
+
   private scheduler: Scheduler | null;
 
   private shouldReload: boolean;
 
+  private deadlineTimer: number;
+
   constructor() {
     super();
     this.active = true;
@@ -63,11 +70,13 @@ export class Manager extends EventEmitter {
       AUTOSAVE_TIMEOUT, this.save.bind(this));
     this.dirty = new CoalescedUpdate(
       DIRTY_TIMEOUT, this.processDirty.bind(this));
+    this.processDeadlines = this.processDeadlines.bind(this);
     this.sids = new Map();
     this.manIds = new Map();
     this.ports = new Set();
     this.scheduler = null;
     this.running = new Set();
+    this.retrying = new Set();
 
     this.startNext = PromiseSerializer.wrapNew(1, this, this.startNext);
 
@@ -188,14 +197,11 @@ export class Manager extends EventEmitter {
     this.notifiedFinished = false;
   }
 
-  async maybeRunFinishActions() {
+  maybeRunFinishActions() {
-    if (this.running.size) {
-      return;
-    }
-    await this.maybeNotifyFinished();
     if (this.running.size) {
       return;
     }
+    this.maybeNotifyFinished();
     if (this.shouldReload) {
       this.saveQueue.trigger();
       setTimeout(() => {
@@ -208,20 +214,15 @@ export class Manager extends EventEmitter {
     setShelfEnabled(true);
   }
 
-  async maybeNotifyFinished() {
+  maybeNotifyFinished() {
-    if (this.notifiedFinished || this.running.size) {
+    if (this.notifiedFinished || this.running.size || this.retrying.size) {
       return;
     }
-    const notification = await Prefs.get("finish-notification", true);
+    if (SOUNDS.value) {
-    const sounds = await Prefs.get("sounds", false);
-    if (this.notifiedFinished || this.running.size) {
-      return;
-    }
-    if (sounds) {
       const audio = new Audio(runtime.getURL("/style/done.opus"));
       audio.addEventListener("canplaythrough", () => audio.play());
     }
-    if (notification) {
+    if (FINISH_NOTIFICATION.value) {
       new Notification(null, _("queue-finished"));
     }
     this.notifiedFinished = true;
@@ -323,6 +324,10 @@ export class Manager extends EventEmitter {
     if (oldState === RUNNING) {
       this.running.delete(download);
     }
+    else if (oldState === RETRYING) {
+      this.retrying.delete(download);
+      this.findDeadline();
+    }
     if (newState === QUEUED) {
       this.resetScheduler();
       this.startNext().catch(console.error);
@@ -334,10 +339,56 @@ export class Manager extends EventEmitter {
       this.running.add(download);
     }
     else {
+      if (newState === RETRYING) {
+        this.addRetry(download);
+      }
       this.startNext().catch(console.error);
     }
   }
 
+  addRetry(download: Download) {
+    this.retrying.add(download);
+    this.findDeadline();
+  }
+
+  private findDeadline() {
+    let deadline = Array.from(this.retrying).
+      reduce<number>((deadline, item) => {
+        if (deadline) {
+          return item.deadline ? Math.min(deadline, item.deadline) : deadline;
+        }
+        return item.deadline;
+      }, 0);
+    if (deadline <= 0) {
+      return;
+    }
+    deadline -= Date.now();
+    if (deadline <= 0) {
+      return;
+    }
+
+    if (this.deadlineTimer) {
+      window.clearTimeout(this.deadlineTimer);
+    }
+    this.deadlineTimer = window.setTimeout(this.processDeadlines, deadline);
+  }
+
+  private processDeadlines() {
+    this.deadlineTimer = 0;
+    try {
+      const now = Date.now();
+      this.items.forEach(item => {
+        if (item.deadline && Math.abs(item.deadline - now) < 1000) {
+          this.retrying.delete(item);
+          item.resume(false);
+        }
+      });
+    }
+    finally {
+      this.findDeadline();
+    }
+  }
+
   sorted(sids: number[]) {
     try {
       // Construct new items

lib/manager/state.ts

@@ -8,8 +8,9 @@ export const PAUSED = 1 << 3;
 export const DONE = 1 << 4;
 export const CANCELED = 1 << 5;
 export const MISSING = 1 << 6;
+export const RETRYING = 1 << 7;
 
-export const RESUMABLE = PAUSED | CANCELED;
+export const RESUMABLE = PAUSED | CANCELED | RETRYING;
-export const FORCABLE = PAUSED | QUEUED | CANCELED;
+export const FORCABLE = PAUSED | QUEUED | CANCELED | RETRYING;
-export const PAUSABLE = QUEUED | CANCELED | RUNNING;
+export const PAUSABLE = QUEUED | CANCELED | RUNNING | RETRYING;
-export const CANCELABLE = QUEUED | RUNNING | PAUSED | DONE | MISSING;
+export const CANCELABLE = QUEUED | RUNNING | PAUSED | DONE | MISSING | RETRYING;

style/common.css

@@ -10,6 +10,7 @@
   --add-color: navy;
   --queue-color: gray;
   --pause-color: #ffa318;
+  --retry-color: rgb(0, 112, 204);
   --error-color: rgb(160, 13, 42);
   --running-color: #aae061;
   --finishing-color: #57cc12;

style/manager.css

@@ -202,6 +202,23 @@ body > * {
   );
 }
 
+.retrying .virtualtable-column-2 .virtualtable-icon {
+  color: var(--retry-color);
+}
+.retrying .virtualtable-column-2 .virtualtable-progress-bar {
+  background: var(--retry-color);
+}
+.retrying .virtualtable-column-2 .virtualtable-progress-undetermined {
+  background: repeating-linear-gradient(
+    45deg,
+    var(--retry-color),
+    var(--retry-color) 6px,
+    transparent 6px,
+    transparent 12px
+  );
+}
+
+
 .missing .virtualtable-column-2 .virtualtable-icon,
 .canceled .virtualtable-column-2 .virtualtable-icon {
   color: var(--error-color);

style/prefs.css

@@ -139,3 +139,10 @@ legend {
   background: rgba(128, 128, 128, 0.05);
   box-shadow: 1px 1px 6px lightgray;
 }
+
+#network-general {
+  display: grid;
+  grid-template-columns: auto 1fr;
+  grid-column-gap: 1em;
+  grid-row-gap: 1ex;
+}

windows/manager/state.ts

@@ -10,6 +10,7 @@ export const StateTexts = locale.then(() => Object.freeze(new Map([
   [DownloadState.QUEUED, _("queued")],
   [DownloadState.RUNNING, _("running")],
   [DownloadState.FINISHING, _("finishing")],
+  [DownloadState.RETRYING, _("paused")],
   [DownloadState.PAUSED, _("paused")],
   [DownloadState.DONE, _("done")],
   [DownloadState.CANCELED, _("canceled")],
@@ -21,6 +22,7 @@ export const StateClasses = Object.freeze(new Map([
   [DownloadState.RUNNING, "running"],
   [DownloadState.FINISHING, "finishing"],
   [DownloadState.PAUSED, "paused"],
+  [DownloadState.RETRYING, "retrying"],
   [DownloadState.DONE, "done"],
   [DownloadState.CANCELED, "canceled"],
   [DownloadState.MISSING, "missing"],
@@ -31,6 +33,7 @@ export const StateIcons = Object.freeze(new Map([
   [DownloadState.RUNNING, "icon-go"],
   [DownloadState.FINISHING, "icon-go"],
   [DownloadState.PAUSED, "icon-pause"],
+  [DownloadState.RETRYING, "icon-pause"],
   [DownloadState.DONE, "icon-done"],
   [DownloadState.CANCELED, "icon-error"],
   [DownloadState.MISSING, "icon-failed"],

windows/manager/table.ts

@@ -146,6 +146,8 @@ export class DownloadItem extends EventEmitter {
 
   public opening: boolean;
 
+  public retries: number;
+
   constructor(owner: DownloadTable, raw: any, stats?: Stats) {
     super();
     Object.assign(this, raw);
@@ -247,6 +249,12 @@ export class DownloadItem extends EventEmitter {
     if (this.state === DownloadState.RUNNING) {
       return this.eta;
     }
+    if (this.state === DownloadState.RETRYING) {
+      if (this.error) {
+        return _("retrying_error", _(this.error) || this.error);
+      }
+      return _("retrying");
+    }
     if (this.error) {
       return _(this.error) || this.error;
     }

windows/prefs.html

@@ -123,9 +123,14 @@
   </article>
 
   <article id="tab-network" class="tab">
-    <fieldset>
+    <fieldset id="network-general">
       <legend data-i18n="pref.netglobal"></legend>
-      <label><span data-i18n="pref-concurrent-downloads">Concurrent downloads</span> <input id="pref-concurrent-downloads" type="number" min="1" max="10"></label>
+      <label data-i18n="pref-concurrent-downloads">Concurrent downloads</label>
+      <input id="pref-concurrent-downloads" type="number" min="1" max="10">
+      <label data-i18n="pref-retries"></label>
+      <input id="pref-retries" type="number" min="0" max="100">
+      <label data-i18n="pref-retry-time"></label>
+      <input id="pref-retry-time" type="number" min="1" max="600">
     </fieldset>
     <table id="limits" data-singleselect="true">
       <tr>

windows/prefs.ts

@@ -623,6 +623,8 @@ addEventListener("DOMContentLoaded", async () => {
 
   // Network
   new IntPref("pref-concurrent-downloads", "concurrent");
+  new IntPref("pref-retries", "retries");
+  new IntPref("pref-retry-time", "retry-time");
 
   visible("#limits").then(() => new LimitsUI());