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

fix: don't exit on reload if there is a syntax error #214

Conversation

alsuren
Copy link
Contributor

@alsuren alsuren commented Dec 21, 2022

fixes #213

Now, if you introduce a syntax error, you immediately get:

 * Detected change in '/Users/david/src/opvia/app/scripts_v3/cloud_function/main.py', reloading
 * Restarting with watchdog (fsevents)
Traceback (most recent call last):
  File "/Users/david/src/opvia/app/.venv/lib/python3.10/site-packages/functions_framework/__init__.py", line 290, in create_app
    spec.loader.exec_module(source_module)
  File "<frozen importlib._bootstrap_external>", line 879, in exec_module
  File "<frozen importlib._bootstrap_external>", line 1017, in get_code
  File "<frozen importlib._bootstrap_external>", line 947, in source_to_code
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/Users/david/src/opvia/app/scripts_v3/cloud_function/main.py", line 11
    ///
    ^^
SyntaxError: invalid syntax
 * Debugger is active!
 * Debugger PIN: 104-682-644

and then if you curl your fuction endpoint, you get the debug html page for the syntax error, and the backtrace gets printed out to stderr again.

If you attempt to start the server when there is already a syntax error present, it correctly exits with the error, as before (the only difference being that the backtrace points at the raise e from None line rather than the spec.loader.exec_module(source_module) line).

This behaviour matches the upstream flask run behaviour now.

Feel free to be brutal about the quality of my python code: the last time I wrote significant production python, it was before python 2.7 was deprecated ;-)

@alsuren alsuren force-pushed the reload-syntax-error-robustness branch from 19a12ad to fcf59c8 Compare December 21, 2022 15:47
@alsuren
Copy link
Contributor Author

alsuren commented Jan 9, 2023

We are now using this patch both locally and in our cloud-function environment. We've not run into any issues with it yet.

What are the next steps for this?

@HKWinterhalter
Copy link
Contributor

It looks like this will need test coverage and linting to pass

@alsuren alsuren force-pushed the reload-syntax-error-robustness branch from 30ce4c9 to 37f3cad Compare September 6, 2023 07:54
@alsuren alsuren force-pushed the reload-syntax-error-robustness branch from 12e4018 to 00ba2cd Compare September 6, 2023 09:31
@alsuren alsuren changed the title don't exit on reload if there is a syntax error fix: don't exit on reload if there is a syntax error Sep 6, 2023
@alsuren
Copy link
Contributor Author

alsuren commented Sep 6, 2023

python -m tox -e lint and python -m tox -e py-macos-latest pass locally (coverage is at 100%)

To work around the "This workflow requires approval from a maintainer" message, I made a PR against my own fork (alsuren#1). The 7 "Expected" checks seem to pass on that PR, so I'm confident that they should pass on this PR too.

I'm assuming that the other tests are not required, or just not configured correctly on my fork. Shout if you need me to look into any more issues.

@HKWinterhalter HKWinterhalter merged commit 46780da into GoogleCloudPlatform:main Sep 6, 2023
42 checks passed
@HKWinterhalter
Copy link
Contributor

Thank you for fixing this @alsuren !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

functions-framework --debug exits if you save with a syntax error
3 participants