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

[Bugfix] Handle missing timing_information gracefully #1640

Conversation

remibaar
Copy link
Contributor

@remibaar remibaar commented May 13, 2021

An error is raised when the devtools before request is not fully executed.
AttributeError: '_AppCtxGlobals' object has no attribute 'timing_information'

I've created a test to replicate, and a fix to handle the error gracefully.

More info in #1624 and #1475

Contributor Checklist

  • I have broken down my PR scope into the following TODO tasks
    • Create test to replicate
    • Fix bug
  • I have run the tests locally and they passed. (refer to testing section in contributing)
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

…` functions has been registered to flask

This test validates issue plotly#1624
The root of this issue lies in the devtools ui, and only occurs when the devtools ui has been enabled
In some situations the `timing_information` is not present in `flask.g`. E.g. when other functions have been registerd to the flask `before_request_funcs`. Dash should anticipate and handle gracefully.

Fixes plotly#1624
@remibaar remibaar changed the title Bugfix/1624 handle missing timing information gracefully [Bugfix] Handle missing timing_information gracefully May 13, 2021
@remibaar
Copy link
Contributor Author

remibaar commented May 14, 2021

Percy check is failing because of a new string "This is a Copy/Paste friendly version of the traceback." But that's (probably) not due to the changes I've made

dash/dash.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Thanks @remibaar - looks good, just a small simplifying suggestion. 💃

You're right that the Percy change has nothing to do with this PR - strange, we must have inherited a new version of Werkzeug or something? Not a big deal.

Co-authored-by: Alex Johnson <johnson.alex.c@gmail.com>
@remibaar
Copy link
Contributor Author

Thanks @alexcjohnson for the simplification! That was exactly how I planned to code it, but for some reason didn't haha. Thanks for catching it.

Do you also agree with the new integration test? I didn't exactly know where to put it, so I've putted in the test_devtools_ui.py

@alexcjohnson
Copy link
Collaborator

The new test is excellent, thanks! I just added a changelog entry, will merge once tests pass.

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.

2 participants