Skip to content

[file_packager] Convert fetchRemotePackage to async. NFC #24893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 1 addition & 4 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -3984,10 +3984,7 @@ def test_file_packager_modularize(self):

create_file('post.js', 'MyModule(Module).then(() => console.log("done"));')

self.run_process([EMCC, 'main.c', '--extern-pre-js=embed.js', '--extern-post-js=post.js', '-sMODULARIZE', '-sEXPORT_NAME=MyModule', '-sFORCE_FILESYSTEM'])

result = self.run_js('a.out.js')
self.assertContained('|hello world|', result)
self.do_runf('main.c', '|hello world|', cflags=['--extern-pre-js=embed.js', '--extern-post-js=post.js', '-sMODULARIZE', '-sEXPORT_NAME=MyModule', '-sFORCE_FILESYSTEM'])

def test_preprocess(self):
# Pass -Werror to prevent regressions such as https://github.com/emscripten-core/emscripten/pull/9661
Expand Down
165 changes: 80 additions & 85 deletions tools/file_packager.py
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,11 @@ def generate_js(data_target, data_files, metadata):

if options.support_node:
ret += " var isNode = typeof process === 'object' && typeof process.versions === 'object' && typeof process.versions.node === 'string';\n"
ret += ' function loadPackage(metadata) {\n'
ret += ' async function loadPackage(metadata) {\n'

code = '''
function assert(check, msg) {
if (!check) throw msg + new Error().stack;
if (!check) throw new Error(msg);
}\n'''

# Set up folders
Expand Down Expand Up @@ -808,7 +808,7 @@ def generate_js(data_target, data_files, metadata):

async function openDatabase() {
if (typeof indexedDB == 'undefined') {
throw 'using IndexedDB to cache data can only be done on a web page or in a web worker';
throw new Error('using IndexedDB to cache data can only be done on a web page or in a web worker');
}
return new Promise((resolve, reject) => {
var openRequest = indexedDB.open(DB_NAME, DB_VERSION);
Expand Down Expand Up @@ -953,66 +953,58 @@ def generate_js(data_target, data_files, metadata):
if options.support_node:
node_support_code = '''
if (isNode) {
require('fs/promises').readFile(packageName).then((contents) => callback(contents.buffer));
return;
var fsPromises = require('fs/promises');
var contents = await fsPromises.readFile(packageName);
return contents.buffer;
}'''.strip()

ret += '''
function fetchRemotePackage(packageName, packageSize, callback) {
async function fetchRemotePackage(packageName, packageSize) {
%(node_support_code)s
Module['dataFileDownloads'] ??= {};
fetch(packageName)
.catch((cause) => Promise.reject(new Error(`Network Error: ${packageName}`, {cause}))) // If fetch fails, rewrite the error to include the failing URL & the cause.
.then((response) => {
if (!response.ok) {
return Promise.reject(new Error(`${response.status}: ${response.url}`));
}

const reader = response.body.getReader();
const iterate = () => reader.read().then(handleChunk).catch((cause) => {
return Promise.reject(new Error(`Unexpected error while handling : ${response.url} ${cause}`, {cause}));
});
try {
var response = await fetch(packageName);
} catch (e) {
throw new Error(`Network Error: ${packageName}`, {e});
}
if (!response.ok) {
throw new Error(`${response.status}: ${response.url}`);
}

const chunks = [];
const headers = response.headers;
const total = Number(headers.get('Content-Length') ?? packageSize);
let loaded = 0;
const chunks = [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Anything here and on can be removed nowadays - all browsers we care about support response.arrayBuffer() so it's enough to return just that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, or is the goal to show actual progress? Then perhaps the opposite, remove the polyfill.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, the goal is to show progress.

I don't know which polyfill is being referred to above, but its not something emscripten includes.

Maybe it can be remove now? It was added in #22016 but I don't see anything about if/when the polyfill might be used. Probably best to leave that to a separate cleanup PR.

const headers = response.headers;
const total = Number(headers.get('Content-Length') ?? packageSize);
let loaded = 0;

const handleChunk = ({done, value}) => {
if (!done) {
chunks.push(value);
loaded += value.length;
Module['dataFileDownloads'][packageName] = {loaded, total};
Module['setStatus']?.('Downloading data...');
const reader = response.body.getReader();

let totalLoaded = 0;
let totalSize = 0;
while (1) {
var {done, value} = await reader.read();
if (done) break;
chunks.push(value);
loaded += value.length;
Module['dataFileDownloads'][packageName] = {loaded, total};

for (const download of Object.values(Module['dataFileDownloads'])) {
totalLoaded += download.loaded;
totalSize += download.total;
}
let totalLoaded = 0;
let totalSize = 0;

Module['setStatus']?.(`Downloading data... (${totalLoaded}/${totalSize})`);
return iterate();
} else {
const packageData = new Uint8Array(chunks.map((c) => c.length).reduce((a, b) => a + b, 0));
let offset = 0;
for (const chunk of chunks) {
packageData.set(chunk, offset);
offset += chunk.length;
}
callback(packageData.buffer);
}
};
for (const download of Object.values(Module['dataFileDownloads'])) {
totalLoaded += download.loaded;
totalSize += download.total;
}

Module['setStatus']?.('Downloading data...');
return iterate();
});
};
Module['setStatus']?.(`Downloading data... (${totalLoaded}/${totalSize})`);
}

function handleError(error) {
console.error('package error:', error);
};\n''' % {'node_support_code': node_support_code}
const packageData = new Uint8Array(chunks.map((c) => c.length).reduce((a, b) => a + b, 0));
let offset = 0;
for (const chunk of chunks) {
packageData.set(chunk, offset);
offset += chunk.length;
}
return packageData.buffer;
}\n''' % {'node_support_code': node_support_code}

code += '''
function processPackageData(arrayBuffer) {
Expand All @@ -1021,7 +1013,7 @@ def generate_js(data_target, data_files, metadata):
var byteArray = new Uint8Array(arrayBuffer);
var curr;
%s
};
}
Module['addRunDependency']('datafile_%s');\n''' % (use_data, js_manipulation.escape_for_js_string(data_target))
# use basename because from the browser's point of view,
# we need to find the datafile in the same dir as the html file
Expand All @@ -1031,32 +1023,31 @@ def generate_js(data_target, data_files, metadata):

if options.use_preload_cache:
code += '''
function preloadFallback(error) {
async function preloadFallback(error) {
console.error(error);
console.error('falling back to default preload behavior');
fetchRemotePackage(REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE, processPackageData);
};

openDatabase()
.then((db) => {
checkCachedPackage(db, PACKAGE_PATH + PACKAGE_NAME)
.then((cachedData) => {
Module['preloadResults'][PACKAGE_NAME] = {fromCache: !!cachedData};
if (cachedData) {
fetchCachedPackage(db, PACKAGE_PATH + PACKAGE_NAME, cachedData).then(processPackageData);
} else {
fetchRemotePackage(REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE,
(packageData) => {
cacheRemotePackage(db, PACKAGE_PATH + PACKAGE_NAME, packageData, {uuid:PACKAGE_UUID})
.then(processPackageData)
.catch((error) => {
console.error(error);
processPackageData(packageData);
});
});
}
})
}).catch(preloadFallback);
processPackageData(await fetchRemotePackage(REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE));
}

try {
var db = await openDatabase();
var pkgMetadata = await checkCachedPackage(db, PACKAGE_PATH + PACKAGE_NAME);
var useCached = !!pkgMetadata;
Module['preloadResults'][PACKAGE_NAME] = {fromCache: useCached};
if (useCached) {
processPackageData(await fetchCachedPackage(db, PACKAGE_PATH + PACKAGE_NAME, pkgMetadata));
} else {
var packageData = await fetchRemotePackage(REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE);
try {
processPackageData(await cacheRemotePackage(db, PACKAGE_PATH + PACKAGE_NAME, packageData, {uuid:PACKAGE_UUID}))
} catch (error) {
console.error(error);
processPackageData(packageData);
}
}
} catch(e) {
await preloadFallback(e);
}

Module['setStatus']?.('Downloading...');\n'''
else:
Expand All @@ -1068,14 +1059,18 @@ def generate_js(data_target, data_files, metadata):
var fetchedCallback = null;
var fetched = Module['getPreloadedPackage'] ? Module['getPreloadedPackage'](REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE) : null;

if (!fetched) fetchRemotePackage(REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE, (data) => {
if (fetchedCallback) {
fetchedCallback(data);
fetchedCallback = null;
} else {
fetched = data;
}
});\n'''
if (!fetched) {
// Note that we don't use await here because we want to execute the
// the rest of this function immediately.
fetchRemotePackage(REMOTE_PACKAGE_NAME, REMOTE_PACKAGE_SIZE).then((data) => {
if (fetchedCallback) {
fetchedCallback(data);
fetchedCallback = null;
} else {
fetched = data;
}
})
}\n'''

code += '''
Module['preloadResults'][PACKAGE_NAME] = {fromCache: false};
Expand All @@ -1087,7 +1082,7 @@ def generate_js(data_target, data_files, metadata):
}\n'''

ret += '''
function runWithFS(Module) {\n'''
async function runWithFS(Module) {\n'''
ret += code
ret += '''
}
Expand Down