Skip to content

Implements Python error hint when importing from src. #5407

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+18 −0
Open

Implements Python error hint when importing from src. #5407

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions src/pyodide/python-entrypoint-helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,22 @@ export function setDoAnImport(
};
}

function handleSrcImport(pyodide: Pyodide, e: any): never {
// Users may be expecting to import local modules via the `src` directory, which for a default
// project structure will fail. This code will add some extra info to the error message to help
// them fix it.
pyodide.runPython(`
import sys
exc = sys.last_value
if exc.name == "src":
Copy link
Contributor

Choose a reason for hiding this comment

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

This will raise an attribute error if exc doesn't have a name attribute. You need to check if it's a ModuleNotFoundError.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the test suite is currently passing let's make sure there's a test where some other error is raised at top level and make sure it doesn't get messed up.

exc.add_note(
"If your main module is inside the 'src' directory then your import " +
"statement shouldn't include a 'src.' prefix")
raise exc
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid memory leaks you should do:

try:
   raise exc
finally:
   del exc

Or ideally move the whole thing into a Python try/catch.
It's important to remember that exceptions hold references to frame objects so keeping them alive also keeps all the local variables from all those scopes alive.

`);
throw e; // Shouldn't reach here, but to keep TS happy we may as well throw it.
}

async function pyimportMainModule(pyodide: Pyodide): Promise<PyModule> {
if (!MAIN_MODULE_NAME.endsWith('.py')) {
throw new PythonUserError(
Expand Down Expand Up @@ -243,6 +259,8 @@ function getMainModule(): Promise<PyModule> {
return await enterJaegerSpan('pyimport_main_module', () =>
pyimportMainModule(pyodide)
);
} catch (e: any) {
handleSrcImport(pyodide, e);
} finally {
Limiter.finishStartup(LOADED_SNAPSHOT_TYPE);
}
Expand Down
Loading