Made the test work in a sensible way

tools/tests/utils-tests.js

@@ -153,14 +153,23 @@ selftest.define("parse url", function () {
   });
 });
 
-// XXX: WIP -- I'm not sure how to properly test a failed connection.
-// At the moment, I'm thinking I'll just mark this test as "don't run"
-// and use it by hand for testing (which basically means run it, kill my
-// internet, start my internet again, watch what happens)
-selftest.define("resume downloads", function () {
+selftest.define("resume downloads", ['net', 'slow'], function () {
+  // A reasonably big file that (I think) should take more than 1s to download
+  // and that we know the size of
   const url = 'http://warehouse.meteor.com/builds/Pr7L8f6PqXyqNJJn4/1443478653127/aRiirNrp4v/meteor-tool-1.1.9-os.osx.x86_64+web.browser+web.cordova.tgz';
 
-  const result = require('../utils/http-helpers').getUrlWithResuming({
+  let firstPass = true;
+  const httpHelpers = require('../utils/http-helpers');
+
+  setTimeout(() => {
+    httpHelpers._currentRequest.emit('error', 'pretend-http-error');
+    httpHelpers._currentRequest.emit('end');
+  }, 1000);
+
+  const result = httpHelpers.getUrlWithResuming({
+    // This doesn't affect the test, but if you remove the timeout above,
+    // you can kill the connection manually by shutting down your network.
+    // This makes it a bit faster
     timeout: 1000,
     url: url,
     encoding: null,
@@ -168,14 +177,14 @@ selftest.define("resume downloads", function () {
     progress: {
       reportProgress({ current, end }) {
         const percent = current / end * 100;
-        if (Math.random() < 0.1) {
-          console.log(`${percent} %`);
+        if (Math.random() < 0.01) {
+          // Uncomment this when manually testing I guess
+          // console.log(`${percent} %`);
         }
       },
-      reportProgressDone() {
-        console.log('done');
-      }
+      reportProgressDone() {}
     }
   });
-  console.log(result, result.toString().length)
+
+  selftest.expectEqual(result.toString().length, 65041076);
 });

tools/utils/http-helpers.js

@@ -15,6 +15,7 @@ var Console = require('../console/console.js').Console;
 var timeoutScaleFactor = require('./utils.js').timeoutScaleFactor;
 
 import { WritableStreamBuffer } from 'stream-buffers';
+import fiberHelpers from '../utils/fiber-helpers.js';
 
 // Helper that tracks bytes written to a writable
 var WritableWithProgress = function (writable, listener) {
@@ -83,6 +84,10 @@ var getUserAgent = function () {
 
 var httpHelpers = exports;
 _.extend(exports, {
+  // For testing purposes, do not use (obviously it doesn't really make
+  // sense to have only one current request)
+  _currentRequest: null,
+
   getUserAgent: getUserAgent,
 
   // A wrapper around request with the following improvements:
@@ -269,6 +274,8 @@ _.extend(exports, {
     var request = require('request');
     var req = request(options, callback);
 
+    // A handle for testing
+    httpHelpers._currentRequest = req;
 
     var totalProgress = { current: 0, end: bodyStreamLength + responseLength, done: false };
 
@@ -372,44 +379,54 @@ _.extend(exports, {
   },
 
   getUrlWithResuming(urlOrOptions) {
-    const options = _.isObject(urlOrOptions) ? urlOrOptions : {
+    const options = _.isObject(urlOrOptions) ? _.clone(urlOrOptions) : {
       url: urlOrOptions,
     };
 
     const outputStream = new WritableStreamBuffer();
 
-    const MAX_ATTEMPTS = 5;
+    const MAX_ATTEMPTS = 10;
+    const RETRY_DELAY_SECS = 5;
+
+    let lastSize = 0;
     function attempt(triesRemaining) {
-      const headers = _.extend({}, urlOrOptions.headers, {
-        range: `bytes=${outputStream.size()}-`,
-      });
+      if (lastSize > 0) {
+        options.headers = {
+          ...options.headers,
+          Range: `bytes=${outputStream.size()}-`
+        };
+      }
+
       try {
         return httpHelpers.request({
           outputStream,
-          headers,
-          ...urlOrOptions,
+          ...options,
         });
       } catch (e) {
-        console.log(e);
-        if (triesRemaining > 0) {
+        const size = outputStream.size();
+        const useTry = size === lastSize;
+        lastSize = outputStream.size();
+
+        if (!useTry || triesRemaining > 0) {
           console.log(`Request failed, retrying`);
-          return attempt(triesRemaining - 1);
+          return new Promise(resolve => {
+            setTimeout(fiberHelpers.bindEnvironment(() => {
+              resolve(attempt(useTry ? triesRemaining - 1 : triesRemaining));
+            }, RETRY_DELAY_SECS * 1000));
+          }).await();
         } else {
           console.log(`Request failed ${MAX_ATTEMPTS} times, failing`)
           throw new files.OfflineError(e);
         }
       }
     }
-    const response = attempt(MAX_ATTEMPTS).response;
-    const body = outputStream.getContents();
 
+    const response = attempt(MAX_ATTEMPTS).response;
     if (response.statusCode >= 400 && response.statusCode < 600) {
       const href = response.request.href;
-      throw Error(
-         body ||
-          `Could not get ${href}; server returned [${response.statusCode}]`);
+      throw Error(`Could not get ${href}; server returned [${response.statusCode}]`);
     } else {
-      return body;
+      return outputStream.getContents();
     }
   }
 });