Skip to content
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.

Sign up for GitHub

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

Jump to bottom

file_packager.py: option to continue-on-error in output js (for optional packages)? #21310

Open
past-due opened this issue Feb 10, 2024 · 4 comments · May be fixed by #21311
Open

file_packager.py: option to continue-on-error in output js (for optional packages)? #21310

past-due opened this issue Feb 10, 2024 · 4 comments · May be fixed by #21311

Comments

@past-due
Copy link
Contributor

Currently, when using file_packager.py to generate a preload package*, the output .js code will throw an error inside fetchRemotePackage if the associated .data file cannot be downloaded. This then yields a state where loading the Module is blocked indefinitely.

* (via file_packager.py --preload [...] --js-output=packageName.js --use-preload-cache)

In the case of optional packages (which an application can handle running without), we'd like to essentially swallow this error and allow loading / running to continue.

The specific use-case: We have optional packages, and an application we're trying to make resilient in offline scenarios (ex. when the .data files for those packages aren't cached). The application can already handle if the optional packages aren't present, so all that's needed is functionality in the file_packager generated script to gracefully handle the error case and remove the run dependencies.

Thoughts on options?

One idea:

Have file_packager.py take a new --continue-on-fetch-error/--continue-on-loading-error/--continue-on-error (?) option that adapts the output .js code to:

  1. Log any error, but either handle or not throw:
    throw new Error("NetworkError for: " + packageName);
    throw new Error(xhr.statusText + " : " + xhr.responseURL);
    (which currently end up as unhandled errors)
  2. (Optionally) Call a special event on Module that could be implemented for external pre/post-js scripts that want to do custom error handling
  3. Remove all run dependencies on error, so that running is unblocked
    Module['removeRunDependency']('datafile_<packageName>.data');
    Module['removeRunDependency'](`fp ${that.name}`); (for all dependencies added as a result of DataRequest.open calls in runWithFS
@past-due past-due changed the title file_packager.py: option to continue-on-error (for optional packages)? file_packager.py: option to continue-on-error in output js (for optional packages)? Feb 10, 2024
@past-due past-due linked a pull request Feb 10, 2024 that will close this issue
Open
@sbc100
Copy link
Collaborator

sbc100 commented Feb 12, 2024

If possible, I'd would prefer not to add more feature to file_package.py itself. Would it be possible to take the generated code and somehow modify/wrap it in try/catch to get the behaviour you want?

If we do end up adding new option perhaps we should just call it --optional?

@past-due
Copy link
Contributor Author

If possible, I'd would prefer not to add more feature to file_package.py itself. Would it be possible to take the generated code and somehow modify/wrap it in try/catch to get the behaviour you want?

@sbc100:

One of the issues encountered investigating such an approach is how to handle unblocking the WebAssembly module.

The file packager's generated script adds multiple run dependencies:

  1. One run dependency based on the packageName (datafile_<packageName>.data) - which is consistent, and could have a corresponding removeRunDependency call hard-coded in external error handling.
  2. Individual run dependencies (`fp ${that.name}`) for every single file that ends up packaged (as a result of every DataRequest.open call via runWithFS).

It appears that 2 is a sticking point (although I may be missing something). And in any event, having to replicate in an external script the internal implementation details of how file_packager.py currently names the run dependencies that it adds... doesn't seem ideal (and seems rather brittle).

(Hence why I went with adding an option to file_packager.py.)

If we do end up adding new option perhaps we should just call it --optional?

I've gone ahead and updated the PR to name it --optional, in case we end up circling back around to this approach.

@sbc100
Copy link
Collaborator

sbc100 commented Feb 12, 2024

Hmm.. I agree that baking in knowledge of the file_package generated code seems bad.

Regarding (2), are you saying that thefp ${that.name} dependencies are per-file? i.e. if you bundle 1000 files it will make 1000 run dependencies?

@past-due
Copy link
Contributor Author

Regarding (2), are you saying that thefp ${that.name} dependencies are per-file? i.e. if you bundle 1000 files it will make 1000 run dependencies?

Yes - for every file, a DataRequest is created and DataRequest.open is called:

emscripten/tools/file_packager.py

Lines 698 to 701 in 52e5687

var files = metadata['files'];
for (var i = 0; i < files.length; ++i) {
new DataRequest(files[i]['start'], files[i]['end'], files[i]['audio'] || 0).open('GET', files[i]['filename']);
}\n''' % (create_preloaded if options.use_preload_plugins else create_data)

And DataRequest.open calls addRunDependency:

emscripten/tools/file_packager.py

Lines 679 to 685 in 52e5687

DataRequest.prototype = {
requests: {},
open: function(mode, name) {
this.name = name;
this.requests[name] = this;
Module['addRunDependency'](`fp ${this.name}`);
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

file_packager.py: Add --optional past-due/emscripten
2 participants
@sbc100 @past-due