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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide Visible Error if <py-env> paths is used in a local HTML file #311

Merged
merged 9 commits into from
May 18, 2022

Conversation

JeffersGlass
Copy link
Member

Add a warning if a user tries to use Paths when opening an HTML file locally instead of using a webserver.

Pyscript Window Error

Specifically, if pyfetch() errors when trying to load a local file (given by paths), a warning will be logged to the console, explaining that a server is necessary to read from local files, and a warning (current a red <div> at the top of the <body> section) will be added to the screen. The console warning starts with 'PyScript' to draw users' (especially new users') attention there.

error-in-console-cropped-justerror2

The method of showing the error is a new function showError() in utils.ts. The styling is very 1995 (see picture above) and should probably change as styling improves, but hopefully this provides a place to start.

Currently, the two errors from pyfetch (Access to fetch at file:///... and GET file:\\\... are not suppressed, though it might be clearer if they were. My understanding is that since pyfetch is a thin wrapper around Fetch, hiding those errors in the console would mean monkey-patching pyfetch()? Or perhaps there's a cleverer way.

error-in-console-cropped2

Resolves #257, and hopefully clarifies other issues listed there.

Copy link
Contributor

@fpliger fpliger left a comment

Choose a reason for hiding this comment

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

@JeffersGlass thanks, this is great! Left some comments that is worth checking/addressing but overall the PR intent lgtm. If you can address what's needed in the comments and check linting (that is causing CI to fail) I'd be happy to merge! 🚀

pyscriptjs/src/components/pyenv.ts Outdated Show resolved Hide resolved
pyscriptjs/src/components/pyenv.ts Outdated Show resolved Hide resolved
pyscriptjs/src/interpreter.ts Outdated Show resolved Hide resolved
pyscriptjs/src/interpreter.ts Show resolved Hide resolved
pyscriptjs/src/interpreter.ts Outdated Show resolved Hide resolved
pyscriptjs/src/utils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@fpliger fpliger left a comment

Choose a reason for hiding this comment

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

@JeffersGlass thanks for the contribution! Nice addition to gracefully fail in case of errors... I've made a couple of changes to merge main into the branch and to reuse the error handling elsewhere as well. TY!

@fpliger fpliger merged commit b767a78 into pyscript:main May 18, 2022
@fpliger fpliger added the tag: component Related to PyScript components label May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: component Related to PyScript components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a better error if <py-env> paths is used in a local HTML file
2 participants