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.

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

Already on GitHub? Sign in to your account

Fails to load Pyodide from external URL in node #3550

Open
ryanking13 opened this issue Feb 6, 2023 · 10 comments
Open

Fails to load Pyodide from external URL in node #3550

ryanking13 opened this issue Feb 6, 2023 · 10 comments
Labels
bug Something isn't working node.js

Comments

@ryanking13
Copy link
Member

ryanking13 commented Feb 6, 2023

馃悰 Bug

It fails to load Pyodide when we set indexURL to some external URL in node.

To Reproduce

npm install pyodide@0.22.1

cat > index.mjs << EOF
import { loadPyodide } from "pyodide";

(async () => await loadPyodide({indexURL: "https://cdn.jsdelivr.net/pyodide/v0.22.1/full/"}))();
EOF

node index.mjs
Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/workspace/temp/nodetest/https:/cdn.jsdelivr.net/pyodide/v0.22.1/full/pyodide.asm.js' imported from /workspace/temp/nodetest/node_modules/pyodide/pyodide.mjs

Environment

  • Pyodide Version: 0.22.1

Additional context

The problem happens in resolvePath

path.resolve that we use in node.js considers URL as a local filesystem path.

> path = require("path");
> path.resolve(".", "https://example.com")
'/workspace/temp/nodetest/https:/example.com'
@ryanking13 ryanking13 added bug Something isn't working node.js labels Feb 6, 2023
@hoodmane
Copy link
Member

hoodmane commented Feb 7, 2023

Well node_resolvePath should return path unchanged if it contains ://. But then there's a problem with __dirname.

@hoodmane
Copy link
Member

hoodmane commented Feb 7, 2023

The problem is here:
https://github.com/emscripten-core/emscripten/blob/main/src/shell.js?plain=1#L223
If I fix that, it successfully fetches pyodide.asm.js but fails to run it with nodeVmMod.runInThisContext with the error:

A dynamic import callback was not specified.

@hoodmane
Copy link
Member

hoodmane commented Feb 7, 2023

Using

nodeVmMod.runInThisContext(result, {async importModuleDynamically(specifier) { return await import(specifier);}})

fixes that error. But now:

Aborted(TypeError: Cannot read properties of undefined (reading 'normalize'))

@hoodmane
Copy link
Member

hoodmane commented Feb 7, 2023

I suppose one option would be to write the url to a file and then import the file, rather than using vm.runInThisContext.

@hoodmane
Copy link
Member

hoodmane commented Feb 7, 2023

We can fix the next error by patching getBinaryPromise to use the ENVIRONMENT_IS_WEB path which fetches the module. But then we get:

Error: ENOENT: no such file or directory, open 'http://0.0.0.0:8003/pyodide.asm.data'
Error: ENOENT: no such file or directory, open 'http://0.0.0.0:8003/repodata.json'

@ryanking13
Copy link
Member Author

Well node_resolvePath should return path unchanged if it contains ://. But then there's a problem with __dirname.

Hmm, I don't get it. node_resolvePath is defined as follows:

nodePath = await import("path");
function node_resolvePath(path: string, base?: string): string {
  return nodePath.resolve(base || ".", path);
}

and it doesn't seems to return path unchanged.

@hoodmane
Copy link
Member

hoodmane commented Feb 7, 2023

Yeah that code should be changed to the following:

function node_resolvePath(path: string, base?: string): string {
  if(path.includes("://")) {
    return path;
  }
  return nodePath.resolve(base || ".", path);
}

But there are lots of problems. I think to fix this we most likely need to patch Emscripten in a couple of places.

@ryanking13
Copy link
Member Author

Oh, thanks. I misunderstood what you said. I naively thought that fixing node_resolvePath would work, but it seems like the problem is more complicated than that.

@hoodmane
Copy link
Member

hoodmane commented Feb 8, 2023

Yeah there just seem to be a lot of places where code expects a URL if in browser and a path if in node. I think we could upstream fixes where needed though.

@savakarrohan
Copy link

I am still facing this issue, any updates for it v0.25.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working node.js
Projects
None yet
Development

No branches or pull requests

3 participants