Merge pull request #3933 from cwillisf/use-scratch-fetch

feat: use scratchFetch

package-lock.json

@@ -25,6 +25,7 @@
         "scratch-sb1-converter": "0.2.7",
         "scratch-translate-extension-languages": "0.0.20191118205314",
         "text-encoding": "0.7.0",
+        "uuid": "8.3.2",
         "worker-loader": "^1.1.1"
       },
       "devDependencies": {
@@ -17485,15 +17486,6 @@
         "node": ">=8"
       }
     },
-    "node_modules/istanbul-lib-processinfo/node_modules/uuid": {
-      "version": "8.3.2",
-      "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
-      "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
-      "dev": true,
-      "bin": {
-        "uuid": "dist/bin/uuid"
-      }
-    },
     "node_modules/istanbul-lib-processinfo/node_modules/which": {
       "version": "2.0.2",
       "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz",
@@ -23874,6 +23866,16 @@
         }
       ]
     },
+    "node_modules/request/node_modules/uuid": {
+      "version": "3.4.0",
+      "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz",
+      "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==",
+      "deprecated": "Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.",
+      "dev": true,
+      "bin": {
+        "uuid": "bin/uuid"
+      }
+    },
     "node_modules/require-directory": {
       "version": "2.1.1",
       "resolved": "https://registry.npmjs.org/require-directory/-/require-directory-2.1.1.tgz",
@@ -30567,13 +30569,11 @@
       }
     },
     "node_modules/uuid": {
-      "version": "3.4.0",
-      "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz",
-      "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==",
-      "deprecated": "Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.",
-      "dev": true,
+      "version": "8.3.2",
+      "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
+      "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
       "bin": {
-        "uuid": "bin/uuid"
+        "uuid": "dist/bin/uuid"
       }
     },
     "node_modules/v8-compile-cache": {
@@ -46012,12 +46012,6 @@
           "integrity": "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==",
           "dev": true
         },
-        "uuid": {
-          "version": "8.3.2",
-          "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
-          "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
-          "dev": true
-        },
         "which": {
           "version": "2.0.2",
           "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz",
@@ -50840,6 +50834,12 @@
           "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz",
           "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==",
           "dev": true
+        },
+        "uuid": {
+          "version": "3.4.0",
+          "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz",
+          "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==",
+          "dev": true
         }
       }
     },
@@ -55859,10 +55859,9 @@
       "dev": true
     },
     "uuid": {
-      "version": "3.4.0",
-      "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.4.0.tgz",
-      "integrity": "sha512-HjSDRw6gZE5JMggctHBcjVak08+KEVhSIiDzFnT9S9aegmp85S/bReBVTb4QTFaRNptJ9kuYaNhnbNEOkbKb/A==",
-      "dev": true
+      "version": "8.3.2",
+      "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
+      "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg=="
     },
     "v8-compile-cache": {
       "version": "2.0.2",

package.json

@@ -58,6 +58,7 @@
     "scratch-sb1-converter": "0.2.7",
     "scratch-translate-extension-languages": "0.0.20191118205314",
     "text-encoding": "0.7.0",
+    "uuid": "8.3.2",
     "worker-loader": "^1.1.1"
   },
   "peerDependencies": {

src/engine/runtime.js

@@ -1,5 +1,6 @@
 const EventEmitter = require('events');
 const {OrderedMap} = require('immutable');
+const uuid = require('uuid');
 
 const ArgumentType = require('../extension-support/argument-type');
 const Blocks = require('./blocks');
@@ -17,6 +18,7 @@ const StageLayering = require('./stage-layering');
 const Variable = require('./variable');
 const xmlEscape = require('../util/xml-escape');
 const ScratchLinkWebSocket = require('../util/scratch-link-websocket');
+const fetchWithTimeout = require('../util/fetch-with-timeout');
 
 // Virtual I/O devices.
 const Clock = require('../io/clock');
@@ -401,6 +403,8 @@ class Runtime extends EventEmitter {
         this.origin = null;
 
         this._initScratchLink();
+
+        this.resetRunId();
     }
 
     /**
@@ -1626,6 +1630,8 @@ class Runtime extends EventEmitter {
      */
     attachStorage (storage) {
         this.storage = storage;
+        fetchWithTimeout.setFetch(storage.scratchFetch.scratchFetch);
+        this.resetRunId();
     }
 
     // -----------------------------------------------------------------------------
@@ -2017,6 +2023,19 @@ class Runtime extends EventEmitter {
         }
     }
 
+    /**
+     * Reset the Run ID. Call this any time the project logically starts, stops, or changes identity.
+     */
+    resetRunId () {
+        if (!this.storage) {
+            // see also: attachStorage
+            return;
+        }
+
+        const newRunId = uuid.v1();
+        this.storage.scratchFetch.setMetadata(this.storage.scratchFetch.RequestMetadata.RunId, newRunId);
+    }
+
     /**
      * Start all threads that start with the green flag.
      */
@@ -2057,6 +2076,8 @@ class Runtime extends EventEmitter {
         }
         // Remove all remaining threads from executing in the next tick.
         this.threads = [];
+
+        this.resetRunId();
     }
 
     /**
@@ -2460,10 +2481,11 @@ class Runtime extends EventEmitter {
     }
 
     /**
-     * Report that the project has loaded in the Virtual Machine.
+     * Handle that the project has loaded in the Virtual Machine.
      */
-    emitProjectLoaded () {
+    handleProjectLoaded () {
         this.emit(Runtime.PROJECT_LOADED);
+        this.resetRunId();
     }
 
     /**

src/extensions/scratch3_text2speech/index.js

@@ -7,7 +7,7 @@ const Cast = require('../../util/cast');
 const MathUtil = require('../../util/math-util');
 const Clone = require('../../util/clone');
 const log = require('../../util/log');
-const fetchWithTimeout = require('../../util/fetch-with-timeout');
+const {fetchWithTimeout} = require('../../util/fetch-with-timeout');
 
 /**
  * Icon svg to be displayed in the blocks category menu, encoded as a data URI.

src/extensions/scratch3_translate/index.js

@@ -2,7 +2,7 @@ const ArgumentType = require('../../extension-support/argument-type');
 const BlockType = require('../../extension-support/block-type');
 const Cast = require('../../util/cast');
 const log = require('../../util/log');
-const fetchWithTimeout = require('../../util/fetch-with-timeout');
+const {fetchWithTimeout} = require('../../util/fetch-with-timeout');
 const languageNames = require('scratch-translate-extension-languages');
 const formatMessage = require('format-message');
 

src/util/fetch-with-timeout.js

@@ -1,3 +1,25 @@
+/**
+ * @callback FetchFunction
+ * @param {RequestInfo|URL} input
+ * @param {RequestInit|undefined} [init]
+ * @returns {Promise<Response>}
+ */
+
+/**
+ * @type {FetchFunction}
+ */
+let myFetch = global.fetch;
+
+/**
+ * Tell `fetchWithTimeout` to use a specific `fetch` function.
+ * By default, `fetchWithTimeout` will use the global `fetch` function.
+ * If there is no global `fetch`, then `fetchWithTimeout` will fail unless provided with an alternative.
+ * @param {FetchFunction} newFetch The new `fetch` function to use within fetchWithTimeout.
+ */
+const setFetch = newFetch => {
+    myFetch = newFetch;
+};
+
 /**
  * Fetch a remote resource like `fetch` does, but with a time limit.
  * @param {Request|string} resource Remote resource to fetch.
@@ -12,7 +34,7 @@ const fetchWithTimeout = (resource, init, timeout) => {
     const signal = controller ? controller.signal : null;
     // The fetch call races a timer.
     return Promise.race([
-        fetch(resource, Object.assign({signal}, init)).then(response => {
+        myFetch(resource, Object.assign({signal}, init)).then(response => {
             clearTimeout(timeoutID);
             return response;
         }),
@@ -25,4 +47,7 @@ const fetchWithTimeout = (resource, init, timeout) => {
     ]);
 };
 
-module.exports = fetchWithTimeout;
+module.exports = {
+    fetchWithTimeout,
+    setFetch
+};

src/virtual-machine.js

@@ -351,7 +351,7 @@ class VirtualMachine extends EventEmitter {
 
         return validationPromise
             .then(validatedInput => this.deserializeProject(validatedInput[0], validatedInput[1]))
-            .then(() => this.runtime.emitProjectLoaded())
+            .then(() => this.runtime.handleProjectLoaded())
             .catch(error => {
                 // Intentionally rejecting here (want errors to be handled by caller)
                 if (error.hasOwnProperty('validationError')) {

test/integration/runId.js

@@ -0,0 +1,73 @@
+const Worker = require('tiny-worker');
+const path = require('path');
+const test = require('tap').test;
+
+const VirtualMachine = require('../../src/index');
+const dispatch = require('../../src/dispatch/central-dispatch');
+
+const makeTestStorage = require('../fixtures/make-test-storage');
+const readFileToBuffer = require('../fixtures/readProjectFile').readFileToBuffer;
+
+// it doesn't really matter which project we use: we're testing side effects of loading any project
+const uri = path.resolve(__dirname, '../fixtures/default.sb3');
+const project = readFileToBuffer(uri);
+
+// By default Central Dispatch works with the Worker class built into the browser. Tell it to use TinyWorker instead.
+dispatch.workerClass = Worker;
+
+test('runId', async t => {
+    const guidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/;
+    const isGuid = data => guidRegex.test(data);
+
+    const storage = makeTestStorage();
+
+    // add to this list every time the RunId should have changed
+    const runIdLog = [];
+    const pushRunId = () => {
+        const runId = storage.scratchFetch.getMetadata(storage.scratchFetch.RequestMetadata.RunId);
+        t.ok(isGuid(runId), 'Run IDs should always be a properly-formatted GUID', {runId});
+        runIdLog.push(runId);
+    };
+
+    const vm = new VirtualMachine();
+    vm.attachStorage(storage);
+    pushRunId(); // check that the initial run ID is valid
+
+    vm.start(); // starts the VM, not the project, so this doesn't change the run ID
+
+    vm.clear();
+    pushRunId(); // clearing the project conceptually changes the project identity does it DOES change the run ID
+
+    vm.setCompatibilityMode(false);
+    vm.setTurboMode(false);
+    await vm.loadProject(project);
+    pushRunId();
+
+    vm.greenFlag();
+    pushRunId();
+
+    // Turn the playgroundData event into a Promise that we can await
+    const playgroundDataPromise = new Promise(resolve => {
+        vm.on('playgroundData', data => resolve(data));
+    });
+
+    // Let the project run for a bit, then get playground data and stop the project
+    // This test doesn't need the playground data but it does need to run & stop the project
+    setTimeout(() => {
+        vm.getPlaygroundData();
+        vm.stopAll();
+        pushRunId();
+    }, 100);
+
+    // wait for the project to run to completion
+    await playgroundDataPromise;
+
+    for (let i = 0; i < runIdLog.length - 1; ++i) {
+        for (let j = i + 1; j < runIdLog.length; ++j) {
+            t.notSame(runIdLog[i], runIdLog[j], 'Run IDs should always be unique', {runIdLog});
+        }
+    }
+
+    vm.quit();
+    t.end();
+});