fix: bad appmaps in django oscar #346
Conversation
zermelo-wisen
force-pushed
the
fix/bad-appmaps-in-django-oscar
branch
4 times, most recently
from
2279c1d
to
0f0a085
Compare
zermelo-wisen
force-pushed
the
fix/bad-appmaps-in-django-oscar
branch
from
0f0a085
to
ec6fe60
Compare
_appmap/recording.py
Outdated
| @@ -81,6 +81,23 @@ def __exit__(self, exc_type, exc_value, tb): | |||
| return False | |||
|
|
|||
|
|
|||
| def check_call_return_stack_order(events): | |||
There was a problem hiding this comment.
We don't want to do this in the agent. AppMaps can be quite large, and this will have a significant performance impact. Detecting the problem during fingerprinting is good enough
There was a problem hiding this comment.
I removed this commit.
(My reasoning was that it would have an overhead but this overhead would always come with the main overhead of creating large AppMaps and it would be relatively small compared to the large AppMap creation itself.)
| # django-oscar commit hash: 9574ce2a56f3d99836c2e20785c116da12af6c42 | ||
| # example test: tests/functional/dashboard/test_dashboard.py | ||
| # TestDashboardIndexForAnonUser::test_is_not_available | ||
| headers = getattr(response, 'headers', getattr(response, '_headers', None)) |
There was a problem hiding this comment.
Why does trying to use headers instead of _headers cause the event-ordering problem?
There was a problem hiding this comment.
While running a django-oscar test, here "AttributeError: 'HttpResponseRedirect' object has no attribute 'headers'" is raised in our code. pytest framework catches it and marks the test as a failed test. We are able to write an AppMap for the failed test after the failed test finishes, as usual. However, our code which creates the http response return event does not get a chance to run because of this exception. This produces a bad AppMap with an http request call event missing a matching http response return event.
- Modifies Django Middleware to work when there is no 'headers'
attribute in the response, but there is a '_headers' attribute
instead.
This issue was observed while running Django Oscar
tests with AppMap Python on a specific version (git commit hash:
9574ce2a56f3d99836c2e20785c116da12af6c42) of Django Oscar.
- Adds a test to ensure that Middleware.__call__ works when
get_response returns a response with a '_headers' attribute
instead of 'headers.'
zermelo-wisen
force-pushed
the
fix/bad-appmaps-in-django-oscar
branch
from
ec6fe60
to
c13cd0c
Compare
Middlewareto work when there is noheadersattribute in the response, but there is a_headersattribute instead. This issue was observed while running Django Oscar tests with AppMap Python on a specific version (git commit hash: 9574ce2a56f3d99836c2e20785c116da12af6c42) of Django Oscar.Middleware.__call__works whenget_responsereturns a response with a_headersattribute instead ofheaders.