Skip to content

Commit bf6d9a9

Browse files
Ashutoshx7Ashutoshx7
authored
convert synchronous XMLHttpRequest to async fetch API (#5454)
* perf: convert synchronous XMLHttpRequest to async fetch API * fix(ProgramBlocks): use correct heap key for oldHeap backup in LoadHeapFromAppBlock The oldHeap backup was incorrectly using logo.turtleHeaps[turtle] (turtle index) instead of logo.turtleHeaps[name] (heap name key). This meant that if a fetch request failed, the rollback would restore data from the wrong heap entry, potentially corrupting state.
1 parent 87360c6 commit bf6d9a9

File tree

3 files changed

+99
-89
lines changed

3 files changed

+99
-89
lines changed

‎js/blocks/ProgramBlocks.js‎

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ function setupProgramBlocks(activity) {
6565

6666
/**
6767
* Executes the flow of the LoadHeapFromAppBlock.
68+
* Uses async fetch to avoid blocking the UI during network requests.
6869
* @param {string[]} args - The arguments passed to the block.
6970
* @param {Object} logo - The logo object.
7071
* @param {Object} turtle - The turtle object.
@@ -76,45 +77,39 @@ function setupProgramBlocks(activity) {
7677
return;
7778
}
7879

79-
let data = [];
8080
const url = args[1];
8181
const name = args[0];
82-
const xmlHttp = new XMLHttpRequest();
83-
let oldHeap = [];
84-
xmlHttp.open("GET", url, false);
85-
xmlHttp.send();
86-
87-
if (xmlHttp.readyState === 4 && xmlHttp.status === 200) {
88-
// eslint-disable-next-line no-console
89-
console.debug(xmlHttp.responseText);
90-
try {
91-
data = JSON.parse(xmlHttp.responseText);
92-
} catch (e) {
82+
const oldHeap = name in logo.turtleHeaps ? logo.turtleHeaps[name] : [];
83+
84+
// Use async fetch to avoid blocking the UI
85+
fetch(url)
86+
.then(response => {
87+
if (!response.ok) {
88+
// eslint-disable-next-line no-console
89+
console.debug("fetched the wrong page or network error...");
90+
activity.errorMsg(_("404: Page not found"));
91+
throw new Error("Network response was not ok");
92+
}
93+
return response.text();
94+
})
95+
.then(responseText => {
9396
// eslint-disable-next-line no-console
94-
console.debug(e);
95-
activity.errorMsg(_("Error parsing JSON data:") + e);
96-
}
97-
} else if (xmlHttp.readyState === 4 && xmlHttp.status !== 200) {
98-
// eslint-disable-next-line no-console
99-
console.debug("fetched the wrong page or network error...");
100-
activity.errorMsg(_("404: Page not found"));
101-
return;
102-
} else {
103-
activity.errorMsg("xmlHttp.readyState: " + xmlHttp.readyState);
104-
return;
105-
}
106-
107-
if (name in logo.turtleHeaps) {
108-
oldHeap = logo.turtleHeaps[turtle];
109-
}
110-
111-
try {
112-
logo.turtleHeaps[name] = data;
113-
} catch (e) {
114-
logo.turtleHeaps[name] = oldHeap;
115-
// eslint-disable-next-line no-console
116-
console.debug(e);
117-
}
97+
console.debug(responseText);
98+
try {
99+
const data = JSON.parse(responseText);
100+
logo.turtleHeaps[name] = data;
101+
} catch (e) {
102+
// eslint-disable-next-line no-console
103+
console.debug(e);
104+
activity.errorMsg(_("Error parsing JSON data:") + e);
105+
logo.turtleHeaps[name] = oldHeap;
106+
}
107+
})
108+
.catch(error => {
109+
// eslint-disable-next-line no-console
110+
console.debug("Fetch error:", error);
111+
logo.turtleHeaps[name] = oldHeap;
112+
});
118113
}
119114
}
120115

‎js/blocks/__tests__/ProgramBlocks.test.js‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -181,54 +181,64 @@ describe("ProgramBlocks", () => {
181181
expect(activity.errorMsg).toHaveBeenCalledWith(NOINPUTERRORMSG, 1);
182182
});
183183

184-
test("loads heap from successful HTTP request", () => {
185-
const mockHttp = {
186-
open: jest.fn(),
187-
send: jest.fn(),
188-
readyState: 4,
189-
status: 200,
190-
responseText: '["item1", "item2"]'
191-
};
192-
global.XMLHttpRequest.mockImplementation(() => mockHttp);
184+
test("loads heap from successful HTTP request", async () => {
185+
global.fetch = jest.fn().mockResolvedValue({
186+
ok: true,
187+
text: jest.fn().mockResolvedValue('["item1", "item2"]')
188+
});
193189

194190
const block = getBlock("loadHeapFromApp");
195191
block.flow(["testHeap", "http://test.com"], logo, 0, 1);
196192

197-
expect(mockHttp.open).toHaveBeenCalledWith("GET", "http://test.com", false);
193+
// Wait for the async fetch to complete
194+
await new Promise(resolve => setTimeout(resolve, 0));
195+
196+
expect(global.fetch).toHaveBeenCalledWith("http://test.com");
198197
expect(logo.turtleHeaps.testHeap).toEqual(["item1", "item2"]);
199198
});
200199

201-
test("handles 404 error", () => {
202-
const mockHttp = {
203-
open: jest.fn(),
204-
send: jest.fn(),
205-
readyState: 4,
206-
status: 404,
207-
responseText: ""
208-
};
209-
global.XMLHttpRequest.mockImplementation(() => mockHttp);
200+
test("handles 404 error", async () => {
201+
global.fetch = jest.fn().mockResolvedValue({
202+
ok: false,
203+
status: 404
204+
});
210205

211206
const block = getBlock("loadHeapFromApp");
212207
block.flow(["testHeap", "http://test.com"], logo, 0, 1);
213208

209+
// Wait for the async fetch to complete
210+
await new Promise(resolve => setTimeout(resolve, 0));
211+
214212
expect(activity.errorMsg).toHaveBeenCalledWith("404: Page not found");
215213
});
216214

217-
test("handles JSON parse error", () => {
218-
const mockHttp = {
219-
open: jest.fn(),
220-
send: jest.fn(),
221-
readyState: 4,
222-
status: 200,
223-
responseText: "invalid json"
224-
};
225-
global.XMLHttpRequest.mockImplementation(() => mockHttp);
215+
test("handles JSON parse error", async () => {
216+
global.fetch = jest.fn().mockResolvedValue({
217+
ok: true,
218+
text: jest.fn().mockResolvedValue("invalid json")
219+
});
226220

227221
const block = getBlock("loadHeapFromApp");
228222
block.flow(["testHeap", "http://test.com"], logo, 0, 1);
229223

224+
// Wait for the async fetch to complete
225+
await new Promise(resolve => setTimeout(resolve, 0));
226+
230227
expect(activity.errorMsg).toHaveBeenCalled();
231228
});
229+
230+
test("handles network error", async () => {
231+
global.fetch = jest.fn().mockRejectedValue(new Error("Network error"));
232+
233+
const block = getBlock("loadHeapFromApp");
234+
block.flow(["testHeap", "http://test.com"], logo, 0, 1);
235+
236+
// Wait for the async fetch to complete
237+
await new Promise(resolve => setTimeout(resolve, 0));
238+
239+
// Should not throw, error is handled gracefully
240+
expect(global.fetch).toHaveBeenCalled();
241+
});
232242
});
233243

234244
describe("SaveHeapToAppBlock", () => {

‎js/utils/utils.js‎

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -224,43 +224,48 @@ function windowWidth() {
224224

225225
/**
226226
* Performs an HTTP GET request to retrieve data from the server.
227+
* Uses async fetch to avoid blocking the UI during network requests.
227228
* @param {string|null} projectName - The name of the project (or null for the base URL).
228-
* @throws {string} Throws an error if the HTTP status code is greater than 299.
229-
* @returns {string} The response text from the server.
229+
* @throws {Error} Throws an error if the HTTP status code is greater than 299.
230+
* @returns {Promise<string>} A promise that resolves to the response text from the server.
230231
*/
231-
let httpGet = projectName => {
232-
let xmlHttp = null;
233-
xmlHttp = new XMLHttpRequest();
234-
if (projectName === null) {
235-
xmlHttp.open("GET", window.server, false);
236-
xmlHttp.setRequestHeader("x-api-key", "3tgTzMXbbw6xEKX7");
237-
} else {
238-
xmlHttp.open("GET", window.server + projectName, false);
239-
xmlHttp.setRequestHeader("x-api-key", "3tgTzMXbbw6xEKX7");
240-
}
232+
let httpGet = async projectName => {
233+
const url = projectName === null ? window.server : window.server + projectName;
234+
const response = await fetch(url, {
235+
method: "GET",
236+
headers: {
237+
"x-api-key": "3tgTzMXbbw6xEKX7"
238+
}
239+
});
241240

242-
xmlHttp.send();
243-
if (xmlHttp.status > 299) {
244-
throw "Error from server";
241+
if (!response.ok) {
242+
throw new Error("Error from server");
245243
}
246244

247-
return xmlHttp.responseText;
245+
return response.text();
248246
};
249247

250248
/**
251249
* Performs an HTTP POST request to send data to the server.
250+
* Uses async fetch to avoid blocking the UI during network requests.
252251
* @param {string} projectName - The name of the project.
253252
* @param {string} data - The data to be sent in the POST request.
254-
* @returns {string} The response text from the server.
253+
* @returns {Promise<string>} A promise that resolves to the response text from the server.
255254
*/
256-
let httpPost = (projectName, data) => {
257-
let xmlHttp = null;
258-
xmlHttp = new XMLHttpRequest();
259-
xmlHttp.open("POST", window.server + projectName, false);
260-
xmlHttp.setRequestHeader("x-api-key", "3tgTzMXbbw6xEKX7");
261-
xmlHttp.send(data);
262-
return xmlHttp.responseText;
263-
// return 'https://apps.facebook.com/turtleblocks/?file=' + projectName;
255+
let httpPost = async (projectName, data) => {
256+
const response = await fetch(window.server + projectName, {
257+
method: "POST",
258+
headers: {
259+
"x-api-key": "3tgTzMXbbw6xEKX7"
260+
},
261+
body: data
262+
});
263+
264+
if (!response.ok) {
265+
throw new Error("Error from server");
266+
}
267+
268+
return response.text();
264269
};
265270

266271
/**

0 commit comments

Comments
 (0)