Opened 6 years ago

Closed 4 years ago

Last modified 3 years ago

#10841 closed New feature (fixed)

Better 500 template for AJAX calls

Reported by: Riz <m.sychev@…> Owned by: SmileyChris
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: alexkoshelev, glenn@…, ramusus@…, justinlilly, RaceCondition, lrekucki@…, kmike84@…, gregor@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently it's quite hard to debug errors raised during ajax calls because of full 500 page being returned. This diff adds simple 500 page, it's pretty close to code for pasting to dpaste but with additional GET\POST\META\FILES\Settings lists and error summary moved to top. Simple check to self.request.is_ajax() is made to select template to use.

As for me, it's much easier to use tools like Firebug with this patch.

Attachments (6)

better_ajax_500.diff (4.3 KB) - added by Riz <overkrik@…> 6 years ago.
better_ajax_500_2.diff (4.3 KB) - added by Michail Sychev <overkrik@…> 6 years ago.
turned off autoescaping for some fields
10841.diff (5.5 KB) - added by SmileyChris 6 years ago.
10841.2.diff (5.5 KB) - added by SmileyChris 6 years ago.
10841.3.diff (8.5 KB) - added by ramiro 4 years ago.
Patch updated to current code status in trunk
10841.4.diff (16.7 KB) - added by ramiro 4 years ago.
Patch updated to account for new 1.4 sensitive POST vars filtering + corresponding tests. Thanks Julien

Download all attachments as: .zip

Change History (36)

Changed 6 years ago by Riz <overkrik@…>

comment:1 Changed 6 years ago by dc

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

Note that you can use DEBUG_PROPAGATE_EXCEPTIONS to get simple traceback.

comment:2 Changed 6 years ago by Michail Sychev <overkrik@…>

I don't think that DEBUG_PROPAGATE_EXCEPTIONS will help. I want django to handle exceptions and provide all info like GET\POST vars, just do it differently in ajax and non ajax cases. And I want to have normal 500 page for non ajax calls.

Changed 6 years ago by Michail Sychev <overkrik@…>

turned off autoescaping for some fields

comment:3 Changed 6 years ago by Michail Sychev <overkrik@…>

New patch, some cleanups.

I think some other templates should have ajax support, like comment's 400 page. But I am not completly sure about it, I use mptt comments which have ajax posting support and having ajax 400 page is very useful. But afaik django default comment system don't have direct ajax support. Any suggestions?

comment:4 Changed 6 years ago by jacob

Why not make the AJAX 500 template be valid JSON? Then it'd be easy for clients to programatically figure out what went wrong.

comment:5 Changed 6 years ago by alexkoshelev

  • Cc alexkoshelev added

comment:6 Changed 6 years ago by Riz

Making it JSON is a nice idea ( maybe with combination of checking "accept" headers ) but afaik when you have 500 error it usually means that something really wrong happend and it's outside the frontend\javascript app logic (syntax error for example, or integrity error), not to note that you need debug 500 page only during development time and adding special logic just for debug time looks like an overkill. So my main point was to make it easy to read by developer, as ( usually ) special logic at javascript side for 500 error page will not exist.

comment:7 Changed 6 years ago by Glenn

  • Cc glenn@… added

This issue is a frustration for everyone doing AJAX with Django. I hope this gets fixed.

I don't think errors should be returned as a broken-apart JSON string. It would make testing with curl, etc. harder, and it doesn't seem useful for clients to try to figure out why the server crashed.

I could see a case for wrapping the entire templated error into JSON, but I think it's simpler for the client to just assume that 500 errors return plaintext, and do something like:

new Ajax.Request("/ajax.json", {
    on500: function(resp) { $("error").update(resp.responseText.replace("\n", "<br>")); }
    onSuccess: function(resp) { f(resp.responseJSON); }
});

... to display errors in the page, even when the normal response is JSON.

comment:8 Changed 6 years ago by Michail Sychev <overkrik@…>

Another point why JSON is bad idea - when you wrap string into JSON all non ascii characters get encoded, which makes it unreadable for normlal user.

comment:9 Changed 6 years ago by SmileyChris

  • Has patch set
  • Triage Stage changed from Unreviewed to Design decision needed
  • Version changed from 1.0 to SVN

Here's a new version which moves the AJAX test to the technical_500_response and returns the correct mime type (I also tidied up the new text version of the template and refactored the html/text traceback generation).

Changed 6 years ago by SmileyChris

comment:10 Changed 6 years ago by SmileyChris

  • Owner changed from nobody to SmileyChris
  • Status changed from new to assigned

(i'm accepting as a reminder for me to raise the issue after 1.1 lands)

Changed 6 years ago by SmileyChris

comment:11 Changed 6 years ago by ramusus

  • Cc ramusus added

comment:12 Changed 6 years ago by justinlilly

  • Cc justinlilly added

comment:13 Changed 6 years ago by ramusus

  • Cc ramusus@… added; ramusus removed

comment:14 Changed 5 years ago by RaceCondition

  • Cc RaceCondition added

comment:15 Changed 5 years ago by Riz

I think this ticket can be closed now as new versions of firebug has "HTML" tab for viewing ajax responses.

comment:16 Changed 5 years ago by RaceCondition

I'm sorry but I personally use Safari/Chrome more often for web development than Firefox, also to inspect AJAX responses.
Does that infer that only Firefox+Firebug are considered as acceptable for development?

comment:17 Changed 5 years ago by ramusus

I'm absolutely agree with RaceCondition

comment:18 Changed 5 years ago by Riz

  • Patch needs improvement set

I am pretty sure that this ticket will not make into Django(lol, it's been a year since first patch) and I have nothing against Chrome/Safari(I had no idea that it can debug AJAX calls to be honest). But now we need a better patch which will server normal html 500 pages to firebug and plain text to other browsers. :)

comment:19 Changed 5 years ago by russellm

Yes, the ticket is a year old - but then, nobody has raised the issue on Django-developers to try and get the design issue resolved, either. Nothing ever gets added to trunk unless somebody makes it happen.

comment:20 Changed 5 years ago by Riz

After looking at headers which firefox send I can seen no way to check for firebug, so we either need to check for firefox and enable full html listing only for it or limit it to text for every browser. Both solutions doesn't seem nice to me, so, any suggestions?

This ticket was discussed in django-developers, btw.

comment:21 Changed 5 years ago by Glenn

By default, failed AJAX requests should give plain-text errors, because very often they're not being viewed in a browser at all. Half of the benefit of the whole mechanism is that I can test it outside of the complexities of whatever HTML interface is using it, by assembling queries with curl. If that fails, it should give messages I can read without having to dump them to a file and load them in a browser (or squint painfully at in a pager, which is what I usually end up doing).

It's nice that some browser tools give a way to view past AJAX responses, but it doesn't change this issue at all.

comment:22 Changed 4 years ago by vdboor

I would love to see JSON in the response, instead of plain text or HTML.
This is what ASP.NET also does, and it works really nice in practice.

Some of the fields could be:

  • errortype
  • message
  • stacktrace

The JavaScript developer can choose whether to show a message, or dump the stracktrace at the console.
Dumping a text/HTML message to the output makes it really hard to do something sane from the JavaScript side of things.

comment:23 Changed 4 years ago by anonymous

I've seen a very nice implementation of something similar to it from Google on their Google Vizualization tools.
Check this out

http://code.google.com/apis/visualization/documentation/dev/implementing_data_source.html#responseformat

It's simliar to waht vdboor suggested.

comment:24 Changed 4 years ago by lrekucki

  • Cc lrekucki@… added

comment:25 Changed 4 years ago by kmike

  • Cc kmike84@… added

comment:26 Changed 4 years ago by SmileyChris

  • Severity set to Normal
  • Type set to New feature

comment:27 Changed 4 years ago by gregmuellegger

  • Cc gregor@… added
  • Easy pickings unset
  • UI/UX unset

comment:28 Changed 4 years ago by ramiro

#16190 and #16227 were duplicates of this one.

comment:29 Changed 4 years ago by anonymous

bump

Changed 4 years ago by ramiro

Patch updated to current code status in trunk

Changed 4 years ago by ramiro

Patch updated to account for new 1.4 sensitive POST vars filtering + corresponding tests. Thanks Julien

comment:30 Changed 4 years ago by ramiro

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

In [16921]:

Fixed #10841 -- Switched response served when DEBUG=True and request.is_ajax() returns True (indicating request has been generated by a JS library) to a plain text version for easier debugging.

Contents of this response are similar to its HTML counterpart modulo frame variables values in the Python traceback section.

Thanks to Riz for the report, to SmileyChris for the patch and to Julien for reviewing.

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