-
Notifications
You must be signed in to change notification settings - Fork 145
Description
Always use FetchTool
in ProxyTool
to workaround worker-based fetch failures
Hi! 👋
Firstly, thanks for your work on this project! 🙂
Background
In our fork of scratch-gui, we currently monkey-patch ProxyTool
in containers/sprite-library.jsx
so that sprite, sound, backdrop and other asset loads bypass the FetchWorkerTool
(which silently fails in our environment) and use FetchTool
instead. We’d like to extend this workaround upstream in scratch-storage so that all asset types load reliably without local monkey-patches.
Problem
By default, ProxyTool
constructs its tool chain like this:
if (filter === ProxyTool.TOOL_FILTER.READY) {
tools = [new FetchTool()];
} else {
tools = [new PublicFetchWorkerTool(), new FetchTool()];
}
That means every non-READY request goes through FetchWorkerTool
first—resulting in silent failures (missing assets, no helpful errors) due to CORS/cookie restrictions and worker initialization timing issues.
Proposed Fix
Simplify ProxyTool
in all build targets to always use only FetchTool
. Here is the real diff applied in our project:
diff --git a/node_modules/scratch-storage/dist/node/scratch-storage.js b/node_modules/scratch-storage/dist/node/scratch-storage.js
index ff3ac05..3a2a396 100644
--- a/node_modules/scratch-storage/dist/node/scratch-storage.js
+++ b/node_modules/scratch-storage/dist/node/scratch-storage.js
@@ -4414,11 +4414,14 @@ class ProxyTool {
let filter = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : ProxyTool.TOOL_FILTER.ALL;
ProxyTool_defineProperty(this, "tools", void 0);
let tools;
- if (filter === ProxyTool.TOOL_FILTER.READY) {
- tools = [new FetchTool()];
- } else {
- tools = [new PublicFetchWorkerTool(), new FetchTool()];
- }
+
+ // Always use FetchTool to avoid worker-based fetch failures:
+ tools = [new FetchTool()];
+ // Legacy:
+ // if (filter === ProxyTool.TOOL_FILTER.READY) {
+ // tools = [new FetchTool()];
+ // } else {
+ // tools = [new PublicFetchWorkerTool(), new FetchTool()];
+ // }
/**
* Sequence of tools to proxy.
* @type {Array.<Tool>}
diff --git a/node_modules/scratch-storage/dist/web/scratch-storage.js b/node_modules/scratch-storage/dist/web/scratch-storage.js
index 1e2aa25..b331317 100644
--- a/node_modules/scratch-storage/dist/web/scratch-storage.js
+++ b/node_modules/scratch-storage/dist/web/scratch-storage.js
@@ -5294,11 +5294,14 @@ class ProxyTool {
let filter = arguments.length > 0 && arguments[0] !== undefined ? arguments[0] : ProxyTool.TOOL_FILTER.ALL;
ProxyTool_defineProperty(this, "tools", void 0);
let tools;
- if (filter === ProxyTool.TOOL_FILTER.READY) {
- tools = [new FetchTool()];
- } else {
- tools = [new PublicFetchWorkerTool(), new FetchTool()];
- }
+
+ // Always use FetchTool to avoid worker-based fetch failures:
+ tools = [new FetchTool()];
+ // Legacy:
+ // if (filter === ProxyTool.TOOL_FILTER.READY) {
+ // tools = [new FetchTool()];
+ // } else {
+ // tools = [new PublicFetchWorkerTool(), new FetchTool()];
+ // }
/**
* Sequence of tools to proxy.
* @type {Array.<Tool>}
diff --git a/node_modules/scratch-storage/src/ProxyTool.ts b/node_modules/scratch-storage/src/ProxyTool.ts
index e6626f4..895430c 100644
--- a/node_modules/scratch-storage/src/ProxyTool.ts
+++ b/node_modules/scratch-storage/src/ProxyTool.ts
@@ -36,11 +36,14 @@ export default class ProxyTool implements Tool {
constructor (filter: ToolFilter = ProxyTool.TOOL_FILTER.ALL) {
let tools: Tool[];
- if (filter === ProxyTool.TOOL_FILTER.READY) {
- tools = [new FetchTool()];
- } else {
- tools = [new FetchWorkerTool(), new FetchTool()];
- }
+
+ // Always use FetchTool to avoid worker-based fetch failures:
+ tools = [new FetchTool()];
+
+ // Legacy:
+ // if (filter === ProxyTool.TOOL_FILTER.READY) {
+ // tools = [new FetchTool()];
+ // } else {
+ // tools = [new FetchWorkerTool(), new FetchTool()];
+ // }
/**
* Sequence of tools to proxy.
* @type {Array.<Tool>}
This patch was initially applied via [patch-package](https://github.com/ds300/patch-package) in our project.
Discussion
- Would it make sense to expose an option in
ProxyTool
to choose between “worker” vs “main-thread” fetch? - Is there a way to preserve
FetchWorkerTool
behind a configuration flag for large-asset performance, while defaulting toFetchTool
?
We’d love to hear your thoughts or suggestions for a more general solution. Thanks!