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

Throw exception if route is not found when in debug mode and add http error views #506

Merged
merged 17 commits into from
Feb 3, 2022

Conversation

girardinsamuel
Copy link
Contributor

@girardinsamuel girardinsamuel commented Jan 21, 2022

Add debug mode (fix #512)

  • controlled by APP_ENV env variable and defined in config/application.py so it can be accessed with config("application.debug")
  • a helper is available: application.is_debug()

When debug mode is on

all exceptions are rendered with exceptionite (except for application/json) fix #487.
image

When debug mode is off

  • all exceptions which are HTTP errors (500, 404, 403) are rendered with a view located in errors/{code}.html. If view does not exist the message is rendered directly as a string with the correct status.
  • all exceptions which have a is_http_exception flag will be rendered like above.
  • any other exception which does not fit here will be rendered as a 500 exception with the page (because when debug is not enable we dont want to see exceptionite page).

Customization

  • HTTP error views can be overriden by overriding .html files
  • For more possibility, you can override the HTTPExceptionHandler to do what you want.

Tests helpers

  • add debugMode context manager to change debug mode locally during a test
  • add env context manager to change env locally during a test

Tinker command

  • now displays environment, debug mode and masonite version for simplicity:
    image

@girardinsamuel girardinsamuel self-assigned this Jan 21, 2022
@girardinsamuel girardinsamuel marked this pull request as draft January 22, 2022 14:49
@girardinsamuel girardinsamuel changed the title Throw exception if route is not found when in debug mode Throw exception if route is not found when in debug mode and add http error views Jan 23, 2022
@girardinsamuel girardinsamuel marked this pull request as ready for review January 23, 2022 15:24
@girardinsamuel
Copy link
Contributor Author

@josephmancuso here is my proposal. ready to discuss it.

src/masonite/foundation/Application.py Outdated Show resolved Hide resolved
src/masonite/providers/RouteProvider.py Show resolved Hide resolved
tests/integrations/config/application.py Show resolved Hide resolved
@girardinsamuel
Copy link
Contributor Author

girardinsamuel commented Jan 23, 2022

I made some updates to the PR it should be good now. We can take many approaches to do this of course... I tried many things and choosed one way.

You can look closely at the updates in ExceptionHandler which is making the switch between debug and not debug and the HttpExceptionHandler which is there to give developers a way to customize things easily (adding call to sentry, or logging stuff or doing anything they want).

@josephmancuso
Copy link
Member

josephmancuso commented Jan 24, 2022

Tested this with APP_DEBUG false and got this error:

  File "/Users/personal/programming/masonite/masonite/src/masonite/exceptions/ExceptionHandler.py", line 54, in handle
    return self.application.make("HttpExceptionHandler").handle(exception)
  File "/Users/personal/programming/masonite/masonite/src/masonite/exceptions/HttpExceptionHandler.py", line 9, in handle
    if self.application.make("view").exists(view_name):
  File "/Users/personal/programming/masonite/masonite/src/masonite/views/view.py", line 150, in exists
    self.load_template(template)
  File "/Users/personal/programming/masonite/masonite/src/masonite/views/view.py", line 224, in load_template
    template_loaders.append(PackageLoader(package_name, package_path))
  File "/Users/personal/Desktop/test 4/venv4/lib/python3.9/site-packages/jinja2/loaders.py", line 319, in __init__
    raise ValueError(
ValueError: The 'templates' package was not installed in a way that PackageLoader understands.
ERROR:waitress:Exception while serving /unkown_route
Traceback (most recent call last):
  File "/Users/personal/programming/masonite/masonite/src/masonite/foundation/response_handler.py", line 27, in response_handler
    application.resolve(provider.boot)
  File "/Users/personal/programming/masonite/masonite/src/masonite/container/container.py", line 172, in resolve
    return obj(*objects)
  File "/Users/personal/programming/masonite/masonite/src/masonite/providers/RouteProvider.py", line 65, in boot
    raise RouteNotFoundException(
masonite.exceptions.exceptions.RouteNotFoundException: GET /unkown_route : 404 Not Found

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/personal/Desktop/test 4/venv4/lib/python3.9/site-packages/waitress/channel.py", line 350, in service
    task.service()
  File "/Users/personal/Desktop/test 4/venv4/lib/python3.9/site-packages/waitress/task.py", line 171, in service
    self.execute()
  File "/Users/personal/Desktop/test 4/venv4/lib/python3.9/site-packages/waitress/task.py", line 441, in execute
    app_iter = self.channel.server.application(environ, start_response)
  File "/Users/personal/programming/masonite/masonite/src/masonite/foundation/Application.py", line 51, in __call__
    return self.response_handler(*args, **kwargs)
  File "/Users/personal/Desktop/test 4/venv4/lib/python3.9/site-packages/whitenoise/base.py", line 85, in __call__
    return self.application(environ, start_response)
  File "/Users/personal/programming/masonite/masonite/src/masonite/foundation/response_handler.py", line 29, in response_handler
    application.make("exception_handler").handle(e)
  File "/Users/personal/programming/masonite/masonite/src/masonite/exceptions/ExceptionHandler.py", line 54, in handle
    return self.application.make("HttpExceptionHandler").handle(exception)
  File "/Users/personal/programming/masonite/masonite/src/masonite/exceptions/HttpExceptionHandler.py", line 9, in handle
    if self.application.make("view").exists(view_name):
  File "/Users/personal/programming/masonite/masonite/src/masonite/views/view.py", line 150, in exists
    self.load_template(template)
  File "/Users/personal/programming/masonite/masonite/src/masonite/views/view.py", line 224, in load_template
    template_loaders.append(PackageLoader(package_name, package_path))
  File "/Users/personal/Desktop/test 4/venv4/lib/python3.9/site-packages/jinja2/loaders.py", line 319, in __init__
    raise ValueError(
ValueError: The 'templates' package was not installed in a way that PackageLoader understands.

@josephmancuso
Copy link
Member

I believe this is due to the fact that I don't have a templates/errors/500.html page

@josephmancuso
Copy link
Member

Everything else seems to work fine

Copy link
Member

@josephmancuso josephmancuso left a comment

Choose a reason for hiding this comment

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

Just need to fix the assumption that the templates errors directory exists

Copy link
Member

@josephmancuso josephmancuso left a comment

Choose a reason for hiding this comment

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

Just need to fix the assumption that the templates errors directory exists

@girardinsamuel
Copy link
Contributor Author

Just need to fix the assumption that the templates errors directory exists

Actually it's more a problem in the view class. The exists() method is not totally right and add_namespaced_location() works for package but does not work for views inside the project itself. We could check that later.
In the meantime, I remove the errors namespace.

Now this works when templates exists or not.

@girardinsamuel
Copy link
Contributor Author

I also added default errors pages in cookie-cutter : MasoniteFramework/cookie-cutter#35
😉
@josephmancuso ready to be merged !

@josephmancuso josephmancuso merged commit be0f202 into MasoniteFramework:4.0 Feb 3, 2022
@girardinsamuel girardinsamuel deleted the fix/487 branch February 3, 2022 08:24
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.

Add missing debug mode Throw exception when in debug mode if route is not found
2 participants