Opened 4 years ago

Closed 2 years ago

#18229 closed New feature (wontfix)

Improving the AJAX Error text version to include the contextual code and variables

Reported by: davehughes05@… Owned by: ANUBHAV JOSHI
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

The plaintext error responses for AJAX requests in 1.4 drop a lot of the stack trace context that was included in the HTML responses pre-1.4. Tools like the network response 'Preview' viewer in Chrome make it easy to navigate the HTML response to drill down into the richer context it provides. The pull request linked below maintains the 1.4 default but enables the response to be configured to use the original behavior.

https://github.com/django/django/pull/8

Attachments (2)

18229.diff (2.9 KB) - added by ANUBHAV JOSHI 2 years ago.
out.diff (2.7 KB) - added by ANUBHAV JOSHI 2 years ago.
Difference in outputs

Download all attachments as: .zip

Change History (20)

comment:1 Changed 4 years ago by davehughes05@…

Needs documentation: set
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 4 years ago by Adrian Holovaty

I agree we should allow this somehow for Chrome, but I'd prefer not to add yet another setting. The barrier to entry for settings is higher. Can you think of another way to solve it?

comment:3 Changed 4 years ago by davehughes05@…

I can't think of an alternative that allows both behaviors without configuring which one is preferred, short of reading the developer's mind :-)

Expanding the returned plaintext to include contextual code and serialized variables (the way they appear in the HTML error result) would probably solve the issue for me. This information could be presented toward the bottom of the trace so the summarizing information is more prominent.

This would likely involve a more in-depth patch, but wouldn't require adding yet another setting. Is this approach worth pursuing, in your opinion?

comment:4 Changed 4 years ago by Adrian Holovaty

Dave -- Sure, improving the text version to include the contextual code and variables sounds like the better plan. I agree that information should appear at the bottom, to present the most important stuff first. Have a go at implementing it!

comment:5 Changed 4 years ago by Jannis Leidel

Triage Stage: UnreviewedAccepted

Marking as accepted as there has apparently been an understanding about the issue already.

comment:6 Changed 3 years ago by Kamu

Has patch: unset

Removing has patch because the pull request was closed in preference of another implementation.

comment:7 Changed 3 years ago by FunkyBob

Which other implementation was chosen?

Is there a separate ticket for it?

Should this be closed as a dupe or fixed?

comment:8 Changed 3 years ago by Tim Graham

Easy pickings: unset
Needs documentation: unset

I think the implementation chosen was "improving the text version to include the contextual code and variables sounds". I don't think it has been implemented yet.

comment:9 Changed 3 years ago by ANUBHAV JOSHI

I hope/intend to work on this in gsoc.

comment:10 Changed 2 years ago by ANUBHAV JOSHI

Owner: changed from nobody to ANUBHAV JOSHI
Status: newassigned
Summary: Allow configuration of debug response format for AJAX errorsImproving the AJAX Error text version to include the contextual code and variables
Version: 1.4master

comment:11 Changed 2 years ago by Tim Graham

As a possible starting point, I've used this middleware before to make it easier to read AJAX exceptions.

comment:12 Changed 2 years ago by ANUBHAV JOSHI

I have compared the two Templates and have added a small diff based on that.

comment:13 Changed 2 years ago by Tim Graham

Has patch: set

comment:14 Changed 2 years ago by ANUBHAV JOSHI

If the changes are good, then I'll open a PR. Also is there a need to update tests here as the text response is not tested that extensively?

comment:15 Changed 2 years ago by Tim Graham

Patch needs improvement: set

The output of the new version is much more cluttered and one of the new lines exceeds 8600 characters in my test. Please post a sample diff of the output between the old and new versions with your next revision, thanks.

Changed 2 years ago by ANUBHAV JOSHI

Attachment: 18229.diff added

Changed 2 years ago by ANUBHAV JOSHI

Attachment: out.diff added

Difference in outputs

comment:16 Changed 2 years ago by Tim Graham

Maybe sniffing the user agent HttpRequest.META['HTTP_USER_AGENT'] and returning the HTML version for Chrome would be a better solution. Actually Firefox inspector and Firebug can also preview HTML. I wonder if we should consider dropping the plain text version for AJAX requests in light of browser improvements? May need a django-developers thread.

comment:17 Changed 2 years ago by Tim Graham

Actually I see the fact that some browsers can preview HTML from AJAX requests was already raised in the original thread, so dropping plain text probably isn't an option.

I'm having trouble coming up with a good way to present the context code and variables that doesn't seem cluttered.

@davehughes05, if you still care about this issue do you have any thoughts?

comment:18 Changed 2 years ago by Tim Graham

Resolution: wontfix
Status: assignedclosed

Closing given lack of follow-up from reporter. If someone feels strongly about this and wants to implement something that isn't too ugly, we'll consider it.

Note: See TracTickets for help on using tickets.
Back to Top