From c2ee02bc2806ecb2acd8e618ad1cbc58732e0d3e Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Fri, 31 Mar 2023 09:06:58 -0700 Subject: [PATCH 1/4] fix(deps): update dependency scratch-storage to v2.2.1 --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 40a9f39d9..07355c583 100644 --- a/package-lock.json +++ b/package-lock.json @@ -57,7 +57,7 @@ "scratch-render": "0.1.0-prerelease.20230318150639", "scratch-render-fonts": "1.0.0-prerelease.20221102164332", "scratch-semantic-release-config": "1.0.7", - "scratch-storage": "2.2.0", + "scratch-storage": "2.2.1", "scratch-svg-renderer": "0.2.0-prerelease.20210727023023", "script-loader": "0.7.2", "semantic-release": "19.0.5", @@ -24373,9 +24373,9 @@ } }, "node_modules/scratch-storage": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.2.0.tgz", - "integrity": "sha512-d2DzApJ9cSlQ42/cEKDD/lfYIocHMskWrOQ5VED6tKgiHirjRZSPflUIfWL8lY5LxE3HSBc/Z7pS3Yvqe/iCGA==", + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.2.1.tgz", + "integrity": "sha512-qogGcWBXqKUHgfvSgyUkos4fuj7z+SDDHBVlT3NNC4gtZgw4dq+USwHjKXCwtRs6BN/joI7+LafFJtSLii6G/w==", "dev": true, "dependencies": { "@babel/runtime": "7.21.0", @@ -51217,9 +51217,9 @@ } }, "scratch-storage": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.2.0.tgz", - "integrity": "sha512-d2DzApJ9cSlQ42/cEKDD/lfYIocHMskWrOQ5VED6tKgiHirjRZSPflUIfWL8lY5LxE3HSBc/Z7pS3Yvqe/iCGA==", + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/scratch-storage/-/scratch-storage-2.2.1.tgz", + "integrity": "sha512-qogGcWBXqKUHgfvSgyUkos4fuj7z+SDDHBVlT3NNC4gtZgw4dq+USwHjKXCwtRs6BN/joI7+LafFJtSLii6G/w==", "dev": true, "requires": { "@babel/runtime": "7.21.0", diff --git a/package.json b/package.json index 0a57290c0..ee5d773d4 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,7 @@ "scratch-render": "0.1.0-prerelease.20230318150639", "scratch-render-fonts": "1.0.0-prerelease.20221102164332", "scratch-semantic-release-config": "1.0.7", - "scratch-storage": "2.2.0", + "scratch-storage": "2.2.1", "scratch-svg-renderer": "0.2.0-prerelease.20210727023023", "script-loader": "0.7.2", "semantic-release": "19.0.5", From 9fca9ee5839a8eec60b1adb6ec3a11638ada0b17 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 13 Mar 2023 11:03:02 -0700 Subject: [PATCH 2/4] feat: use scratchFetch BREAKING CHANGE: servers must accept new header x-projectid --- package-lock.json | 49 +++++++++++++++++----------------- package.json | 1 + src/engine/runtime.js | 20 ++++++++++++-- src/util/fetch-with-timeout.js | 4 ++- src/virtual-machine.js | 2 +- 5 files changed, 47 insertions(+), 29 deletions(-) diff --git a/package-lock.json b/package-lock.json index 07355c583..0b76d3fbc 100644 --- a/package-lock.json +++ b/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": { @@ -17463,15 +17464,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", @@ -23846,6 +23838,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", @@ -30520,13 +30522,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": { @@ -45947,12 +45947,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", @@ -50769,6 +50763,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 } } }, @@ -55771,10 +55771,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", diff --git a/package.json b/package.json index ee5d773d4..c5940c35d 100644 --- a/package.json +++ b/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": { diff --git a/src/engine/runtime.js b/src/engine/runtime.js index d79337caf..1f29bb25d 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1,5 +1,8 @@ const EventEmitter = require('events'); const {OrderedMap} = require('immutable'); +const uuid = require('uuid'); + +const {setMetadata, RequestMetadata} = require('scratch-storage/src/scratchFetch.js'); const ArgumentType = require('../extension-support/argument-type'); const Blocks = require('./blocks'); @@ -401,6 +404,8 @@ class Runtime extends EventEmitter { this.origin = null; this._initScratchLink(); + + this.resetRunId(); } /** @@ -2015,6 +2020,14 @@ class Runtime extends EventEmitter { } } + /** + * Reset the Run ID. Call this any time the project logically starts, stops, or changes identity. + */ + resetRunId () { + const newRunId = uuid.v1(); + setMetadata(RequestMetadata.RunId, newRunId); + } + /** * Start all threads that start with the green flag. */ @@ -2055,6 +2068,8 @@ class Runtime extends EventEmitter { } // Remove all remaining threads from executing in the next tick. this.threads = []; + + this.resetRunId(); } /** @@ -2458,10 +2473,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(); } /** diff --git a/src/util/fetch-with-timeout.js b/src/util/fetch-with-timeout.js index 983c888b3..80c1b11f0 100644 --- a/src/util/fetch-with-timeout.js +++ b/src/util/fetch-with-timeout.js @@ -1,3 +1,5 @@ +const {scratchFetch} = require('scratch-storage/src/scratchFetch.js'); + /** * Fetch a remote resource like `fetch` does, but with a time limit. * @param {Request|string} resource Remote resource to fetch. @@ -12,7 +14,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 => { + scratchFetch(resource, Object.assign({signal}, init)).then(response => { clearTimeout(timeoutID); return response; }), diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 8094efbf8..6693a1f3a 100644 --- a/src/virtual-machine.js +++ b/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')) { From e696e3548b542f37fe800d7564c1d908777a0935 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Mon, 27 Mar 2023 10:31:07 -0700 Subject: [PATCH 3/4] fix: don't set run ID until storage exists (also add tests) --- src/engine/runtime.js | 10 ++++-- test/integration/runId.js | 73 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 test/integration/runId.js diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 1f29bb25d..87b44d0d3 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -2,8 +2,6 @@ const EventEmitter = require('events'); const {OrderedMap} = require('immutable'); const uuid = require('uuid'); -const {setMetadata, RequestMetadata} = require('scratch-storage/src/scratchFetch.js'); - const ArgumentType = require('../extension-support/argument-type'); const Blocks = require('./blocks'); const BlocksRuntimeCache = require('./blocks-runtime-cache'); @@ -1629,6 +1627,7 @@ class Runtime extends EventEmitter { */ attachStorage (storage) { this.storage = storage; + this.resetRunId(); } // ----------------------------------------------------------------------------- @@ -2024,8 +2023,13 @@ 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(); - setMetadata(RequestMetadata.RunId, newRunId); + this.storage.scratchFetch.setMetadata(this.storage.scratchFetch.RequestMetadata.RunId, newRunId); } /** diff --git a/test/integration/runId.js b/test/integration/runId.js new file mode 100644 index 000000000..cdc006b20 --- /dev/null +++ b/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(); +}); From 829ae1dface2890e6dc8d8f39b62a3180d006853 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 28 Mar 2023 13:20:58 -0700 Subject: [PATCH 4/4] fix: tell fetchWithTimeout to use scratchFetch from storage instance --- src/engine/runtime.js | 2 ++ src/extensions/scratch3_text2speech/index.js | 2 +- src/extensions/scratch3_translate/index.js | 2 +- src/util/fetch-with-timeout.js | 29 ++++++++++++++++++-- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 87b44d0d3..2c5f8eef2 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -18,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'); @@ -1627,6 +1628,7 @@ class Runtime extends EventEmitter { */ attachStorage (storage) { this.storage = storage; + fetchWithTimeout.setFetch(storage.scratchFetch.scratchFetch); this.resetRunId(); } diff --git a/src/extensions/scratch3_text2speech/index.js b/src/extensions/scratch3_text2speech/index.js index f45c7010b..8c76a6a11 100644 --- a/src/extensions/scratch3_text2speech/index.js +++ b/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. diff --git a/src/extensions/scratch3_translate/index.js b/src/extensions/scratch3_translate/index.js index 871ca326a..3964711fc 100644 --- a/src/extensions/scratch3_translate/index.js +++ b/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'); diff --git a/src/util/fetch-with-timeout.js b/src/util/fetch-with-timeout.js index 80c1b11f0..465ce1444 100644 --- a/src/util/fetch-with-timeout.js +++ b/src/util/fetch-with-timeout.js @@ -1,4 +1,24 @@ -const {scratchFetch} = require('scratch-storage/src/scratchFetch.js'); +/** + * @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. @@ -14,7 +34,7 @@ const fetchWithTimeout = (resource, init, timeout) => { const signal = controller ? controller.signal : null; // The fetch call races a timer. return Promise.race([ - scratchFetch(resource, Object.assign({signal}, init)).then(response => { + myFetch(resource, Object.assign({signal}, init)).then(response => { clearTimeout(timeoutID); return response; }), @@ -27,4 +47,7 @@ const fetchWithTimeout = (resource, init, timeout) => { ]); }; -module.exports = fetchWithTimeout; +module.exports = { + fetchWithTimeout, + setFetch +};