Skip to content
Closed
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
87743d1
Adding USE_FETCH setting.
seanmorris May 25, 2024
3d38841
Tweaks
seanmorris May 25, 2024
ffcaa3b
Adding tests.
seanmorris May 26, 2024
de79ead
Tweak.
seanmorris May 26, 2024
139bcd4
Tweak.
seanmorris May 26, 2024
893f361
Merge branch 'main' into sm-worker-fetch
seanmorris May 26, 2024
0407112
Tweak.
seanmorris May 26, 2024
24761b6
Merge branch 'sm-worker-fetch' of github.com:seanmorris/emscripten in…
seanmorris May 26, 2024
3ead8de
Rebasing generated sizes.
seanmorris May 26, 2024
35b593f
Reverting extraneous change.
seanmorris May 26, 2024
5c2a7bb
Readability.
seanmorris May 26, 2024
5631928
Testing
seanmorris May 26, 2024
ff2d142
Removing debug.
seanmorris May 26, 2024
9e06ef2
Typo.
seanmorris May 26, 2024
5899d3f
Parameterizing test.
seanmorris May 26, 2024
c09f2c5
Spacing.
seanmorris May 26, 2024
2f43c20
Deduping default info in docs.
seanmorris May 26, 2024
c5ecce5
Deduping if statement.
seanmorris May 26, 2024
7e86b5b
Correcting indentation of JS output.
seanmorris May 27, 2024
6848055
Removing use_fetch option and XmlHttpRequest.
seanmorris May 29, 2024
0bf9b71
Reverting extraneous changes.
seanmorris May 29, 2024
3f765bc
Revert "Reverting extraneous changes."
seanmorris May 29, 2024
316aa8b
Actually reverting extraneous change.
seanmorris May 29, 2024
366b942
Merge branch 'sm-worker-fetch' into updates
seanmorris May 29, 2024
58ab5c6
Merging main
seanmorris May 29, 2024
5899e42
Throw error on bad fetch in runMetaWithFS
seanmorris May 29, 2024
92ffd13
Tweaks.
seanmorris May 29, 2024
df4e1c3
Tweaks.
seanmorris May 29, 2024
bb9a69e
Use rejecting promises instead of throw.
seanmorris May 29, 2024
c8e9db9
Merge branch 'main' into sm-worker-fetch
seanmorris May 29, 2024
272f093
Tweaks.
seanmorris May 29, 2024
6432c5a
Adding polyfill + test.
seanmorris May 29, 2024
e870667
Adding polyfill + test.
seanmorris May 29, 2024
ce4089e
Adding polyfill + test.
seanmorris May 29, 2024
4581d90
Adding fetch polyfill + test
seanmorris May 29, 2024
457ef2c
Adding fetch polyfill + test
seanmorris May 29, 2024
aeac94e
Polyfill tests for browser.
seanmorris May 30, 2024
275388a
Tweaks.
seanmorris May 30, 2024
d4a6162
Tweaks.
seanmorris May 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions src/web_or_worker_shell_read.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,13 @@
}

readAsync = (url, onload, onerror) => {
var xhr = new XMLHttpRequest();
xhr.open('GET', url, true);
xhr.responseType = 'arraybuffer';
xhr.onload = () => {
if (xhr.status == 200 || (xhr.status == 0 && xhr.response)) { // file URLs can return 0
onload(xhr.response);
return;
fetch(url)
.then(response => {
if(response.ok) {
return response.arrayBuffer();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not call onload here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

response.arrayBuffer(); returns a promise that resolves to a buffer, not a literal buffer.

}
onerror();
};
xhr.onerror = onerror;
xhr.send(null);
}

throw new Error(response.statusText + ' : ' + response.url);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not call onerror instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check the make sure the error has the correct stack if its not actually thrown, and then I can change it.

Copy link
Contributor Author

@seanmorris seanmorris May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remembered why I used this pattern as I was editing: The catch() at the end of the function will catch errors coming from the fetch(), but fetch() won't throw errors by itself if the request fails, i.e. in a 404 or 500 error. For that we have to throw our own error. When we do that, we skip the then() on line 32 and head right to catch().

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about return new Promise.reject(..)

Then I think maybe you can do .then(onload, onerror) without the extra catch? Not sure which is more idomatic.

})
.then(onload)
.catch(onerror)
};
2 changes: 1 addition & 1 deletion test/other/test_unoptimized_code_size_no_asserts.js.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
31420
31418
6 changes: 4 additions & 2 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -699,15 +699,17 @@ def setup(assetLocalization):
<center><canvas id='canvas' width='256' height='256'></canvas></center>
<hr><div id='output'></div><hr>
<script type='text/javascript'>
window.onerror = (error) => {
const handler = event => {
const error = String(event.reason);
window.disableErrorReporting = true;
window.onerror = null;
var result = error.includes("test.data") ? 1 : 0;
var xhr = new XMLHttpRequest();
xhr.open('GET', 'http://localhost:8888/report_result?' + result, true);
xhr.send();
setTimeout(function() { window.close() }, 1000);
}
};
window.addEventListener('unhandledrejection', handler);
var Module = {
locateFile: function (path, prefix) {if (path.endsWith(".wasm")) {return prefix + path;} else {return "''' + assetLocalization + r'''" + path;}},
print: (function() {
Expand Down
121 changes: 64 additions & 57 deletions tools/file_packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ def generate_object_file(data_files):

def main():
if len(sys.argv) == 1:
err('''Usage: file_packager TARGET [--preload A [B..]] [--embed C [D..]] [--exclude E [F..]]] [--js-output=OUTPUT.js] [--no-force] [--use-preload-cache] [--indexedDB-name=EM_PRELOAD_CACHE] [--separate-metadata] [--lz4] [--use-preload-plugins]
err('''Usage: file_packager TARGET [--preload A [B..]] [--embed C [D..]] [--exclude E [F..]]] [--js-output=OUTPUT.js] [--no-force] [--use-preload-cache] [--indexedDB-name=EM_PRELOAD_CACHE] [--separate-metadata] [--lz4] [--use-preload-plugins] [--no-node]
See the source for more details.''')
return 1

Expand Down Expand Up @@ -775,7 +775,7 @@ def generate_js(data_target, data_files, metadata):
}
var REMOTE_PACKAGE_NAME = Module['locateFile'] ? Module['locateFile'](REMOTE_PACKAGE_BASE, '') : REMOTE_PACKAGE_BASE;\n''' % (js_manipulation.escape_for_js_string(data_target), js_manipulation.escape_for_js_string(remote_package_name))
metadata['remote_package_size'] = remote_package_size
ret += '''var REMOTE_PACKAGE_SIZE = metadata['remote_package_size'];\n'''
ret += ''' var REMOTE_PACKAGE_SIZE = metadata['remote_package_size'];\n'''

if options.use_preload_cache:
# Set the id to a hash of the preloaded data, so that caches survive over multiple builds
Expand Down Expand Up @@ -965,59 +965,68 @@ def generate_js(data_target, data_files, metadata):
});
return;
}'''.strip()

ret += '''
function fetchRemotePackage(packageName, packageSize, callback, errback) {
%(node_support_code)s
var xhr = new XMLHttpRequest();
xhr.open('GET', packageName, true);
xhr.responseType = 'arraybuffer';
xhr.onprogress = function(event) {
var url = packageName;
var size = packageSize;
if (event.total) size = event.total;
if (event.loaded) {
if (!xhr.addedTotal) {
xhr.addedTotal = true;
if (!Module.dataFileDownloads) Module.dataFileDownloads = {};
Module.dataFileDownloads[url] = {
loaded: event.loaded,
total: size
};
} else {
Module.dataFileDownloads[url].loaded = event.loaded;
Module.dataFileDownloads = Module.dataFileDownloads || {};
const url = packageName;
fetch(url)
.catch(error => { throw new Error(error + ' : ' + url) }) // Do this first so we don't catch errors from then()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of .catch here is it the same to write the error handler as the second argument to then()? Is one way preferable over the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, we need to catch() before then() so we only handle fetch errors here. If we putt this at the bottom, or in the second param of this(), then it will also act on the network errors generated by then(), and I'd need to add logic to skip certain errors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our style uses braces around the single argument to arrow functions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw isn't a function, so it needs to have braces or it counts as a syntax error. I'm switching to Promise.reject which is a function.

.then(response => {

let loaded = 0;

const reader = response.body.getReader();
const headers = response.headers;

if (!response.ok) {
throw new Error(response.statusText + ' : ' + response.url);
}

const total = headers.get('Content-Length') ?? packageSize;
const chunks = [];

const iterate = () => reader.read().then(handleChunk).catch(cause => {
throw new Error(response.statusText + ' : ' + response.url, {cause});
});

const handleChunk = ({done, value}) => {
if (!done) {
chunks.push(value);
loaded += value.length;
Module.dataFileDownloads[url] = Module.dataFileDownloads[url] ?? {};
Module.dataFileDownloads[url].loaded = loaded;
Module.dataFileDownloads[url].total = total;

if (total) {
if (Module['setStatus']) Module['setStatus'](`Downloading data... (${loaded}/${total})`);
}
else {
if (Module['setStatus']) Module['setStatus']('Downloading data...');
}
return iterate();
}
var total = 0;
var loaded = 0;
var num = 0;
for (var download in Module.dataFileDownloads) {
var data = Module.dataFileDownloads[download];
total += data.total;
loaded += data.loaded;
num++;
else {
const size = chunks.map(c => c.length).reduce((a, b) => a + b, 0);
let index = 0;
const packageData = new Uint8Array(size);
for(const chunk of chunks) {
packageData.set(chunk, index);
index += chunk.length;
}

callback(packageData.buffer);
}
total = Math.ceil(total * Module.expectedDataFileDownloads/num);
if (Module['setStatus']) Module['setStatus'](`Downloading data... (${loaded}/${total})`);
} else if (!Module.dataFileDownloads) {
if (Module['setStatus']) Module['setStatus']('Downloading data...');
}
};
xhr.onerror = function(event) {
throw new Error("NetworkError for: " + packageName);
}
xhr.onload = function(event) {
if (xhr.status == 200 || xhr.status == 304 || xhr.status == 206 || (xhr.status == 0 && xhr.response)) { // file URLs can return 0
var packageData = xhr.response;
callback(packageData);
} else {
throw new Error(xhr.statusText + " : " + xhr.responseURL);
}
};
xhr.send(null);
};
};
return iterate();
});
};\n''' % {'node_support_code': node_support_code}

ret += '''
function handleError(error) {
console.error('package error:', error);
};\n''' % {'node_support_code': node_support_code}
};\n'''

code += '''
function processPackageData(arrayBuffer) {
Expand Down Expand Up @@ -1113,15 +1122,14 @@ def generate_js(data_target, data_files, metadata):
function runMetaWithFS() {
Module['addRunDependency']('%(metadata_file)s');
var REMOTE_METADATA_NAME = Module['locateFile'] ? Module['locateFile']('%(metadata_file)s', '') : '%(metadata_file)s';
var xhr = new XMLHttpRequest();
xhr.onreadystatechange = function() {
if (xhr.readyState === 4 && xhr.status === 200) {
loadPackage(JSON.parse(xhr.responseText));
}
}
xhr.open('GET', REMOTE_METADATA_NAME, true);
xhr.overrideMimeType('application/json');
xhr.send(null);
fetch(REMOTE_METADATA_NAME)
.then(response => {
if(response.ok) {
return response.json();
}
throw new Error(response.statusText + ' : ' + response.url);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would return Promise.reject(new Error(response.statusText + ' : ' + response.url)) instead of throwing?

})
.then(loadPackage);
}

if (Module['calledRun']) {
Expand All @@ -1130,7 +1138,6 @@ def generate_js(data_target, data_files, metadata):
if (!Module['preRun']) Module['preRun'] = [];
Module["preRun"].push(runMetaWithFS);
}\n''' % {'metadata_file': os.path.basename(options.jsoutput + '.metadata')}

else:
_metadata_template = '''
}
Expand Down