Opened 3 years ago

Closed 15 months ago

#18229 closed New feature (wontfix)

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

Reported by: davehughes05@… Owned by: anubhav9042
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 anubhav9042 15 months ago.
out.diff (2.7 KB) - added by anubhav9042 15 months ago.
Difference in outputs

Download all attachments as: .zip

Change History (20)

comment:1 Changed 3 years ago by davehughes05@…

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

comment:2 Changed 3 years ago by adrian

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 3 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 3 years ago by adrian

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 3 years ago by jezdez

  • Triage Stage changed from Unreviewed to Accepted

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

comment:6 Changed 2 years ago by Kamu

  • Has patch unset

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

comment:7 Changed 2 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 2 years ago by timo

  • 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 18 months ago by anubhav9042

I hope/intend to work on this in gsoc.

comment:10 Changed 15 months ago by anubhav9042

  • Owner changed from nobody to anubhav9042
  • Status changed from new to assigned
  • Summary changed from Allow configuration of debug response format for AJAX errors to Improving the AJAX Error text version to include the contextual code and variables
  • Version changed from 1.4 to master

comment:11 Changed 15 months ago by timo

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

comment:12 Changed 15 months ago by anubhav9042

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

comment:13 Changed 15 months ago by timo

  • Has patch set

comment:14 Changed 15 months ago by anubhav9042

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 15 months ago by timo

  • 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 15 months ago by anubhav9042

Changed 15 months ago by anubhav9042

Difference in outputs

comment:16 Changed 15 months ago by timo

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 15 months ago by timo

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 15 months ago by timo

  • Resolution set to wontfix
  • Status changed from assigned to closed

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