Opened 5 years ago

Closed 5 years ago

#25646 closed New feature (duplicate)

Sort the `local vars` in the rich exception reporter

Reported by: Keryn Knight Owned by:
Component: Error reporting Version: master
Severity: Normal Keywords:
Cc: django@…, Andrew Kuchev Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Currently, I think the locals for each stack in the technical 500 template (and email) are ordered by ... their order in the source, maybe? I'm not actually sure, but it's probably either that, or is just plain non-deterministic.

I think it'd be nicer if the locals were instead sorted by their name, alphabetically, such that you can reasonably approximate where to look in the expanded element.

For example, where currently I see:

e, callback_args, resolver_match, middleware_method, request, wrapped_callback, resolver, callback_kwargs

all listed with their values, I'd prefer to see:

callback_args, callback_kwargs, e, middleware_method, request, resolver, resolver_match, wrapped_callback

It might also be nice to change the title for each frame from Local vars to <N> Local vars (though I'd prefer vars become variables tbh) so that its clearer how much information might be gleaned from inspecting a stack, and how might work it might do.

I believe that the only change necessary to fix the sorting is to change

frame['vars'] = frame_vars

to

frame['vars'] = sorted(frame_vars, key=itemgetter(0))

Change History (7)

comment:1 Changed 5 years ago by Keryn Knight

Cc: django@… added

comment:2 Changed 5 years ago by Tim Graham

Easy pickings: set
Triage Stage: UnreviewedAccepted

comment:3 Changed 5 years ago by Andrew Kuchev

Cc: Andrew Kuchev added

I was going to take this issue, however, I noticed, that variables "should be" already sorted in the DEBUG page, because there is a dictsort call inside template code (line 816, file django/views/debug.py):

{% for var in frame.vars|dictsort:"0" %}

However, this call actually does nothing. You may use the following test case:

def test_sort_list_of_tuples(self):
    data = [('a', '42'), ('c', 'string'), ('b', 'foo')]
    sorted_data = dictsort(data, '0')

    self.assertEqual([('a', '42'), ('b', 'foo'), ('c', 'string')], sorted_data)

And this is the output:

Traceback (most recent call last):
  File ".../django/tests/template_tests/filter_tests/test_dictsort.py", line 40, in test_sort_list_of_tuples
    self.assertEqual([('a', '42'), ('b', 'foo'), ('c', 'string')], sorted_data)
AssertionError: Lists differ: [('a', '42'), ('b', 'foo'), ('c', 'string')] != [('a', '42'), ('c', 'string'), ('b', 'foo')]

First differing element 1:
('b', 'foo')
('c', 'string')

- [('a', '42'), ('b', 'foo'), ('c', 'string')]
+ [('a', '42'), ('c', 'string'), ('b', 'foo')]

So, I wonder, if this behavior of dictsort is correct (according to doc comment, the filter accepts a list of dicts, not a list of tuples), or not (well, any dict may be presented as list of tuples)?

comment:4 Changed 5 years ago by Baptiste Mispelon

You might be on to something.

I did a quick regression testing and it seems that sorting of tuples worked in 1.2 but was broken in 1.3 by 561af6417e1c8232904b726fb0219cc0b7c2e71d (though I don't really understand how this particular commit could have broken it).

dictsort is used in a few other places in the debug view and I think all of them are also broken.

Considering this, I think the best approach would be to restore the functionality of sorting tuples, test it and document it.
What do you think?

comment:5 in reply to:  4 Changed 5 years ago by Andrew Kuchev

Replying to bmispelon:

You might be on to something.

I did a quick regression testing and it seems that sorting of tuples worked in 1.2 but was broken in 1.3 by 561af6417e1c8232904b726fb0219cc0b7c2e71d (though I don't really understand how this particular commit could have broken it).

Seems like Variable('0').resolve(context) returns 0 regardless of given context, that is why sorting by key=Variable('0').resolve does not work. Transferring each item into a tuple of (var_resolve(item), item) allows us to order tuples by their second element, if the first elements are equal. However, this approach will fail, when the item is of unorderable type (see the test case below).

Note also: function dictsort will fail for dictionaries with numeric keys :

def test_sort_list_of_tuple_like_dicts(self):
    data = [{'0': 'a', '1': '42'},
            {'0': 'c', '1': 'string'},
            {'0': 'b', '1': 'foo'}]

    sorted_data = dictsort(data, '0')

    self.assertEqual([{'0': 'a', '1': '42'},
                      {'0': 'b', '1': 'foo'},
                      {'0': 'c', '1': 'string'}], sorted_data)

dictsort is used in a few other places in the debug view and I think all of them are also broken.

Considering this, I think the best approach would be to restore the functionality of sorting tuples, test it and document it.
What do you think?

Yes, I sure, this is the right approach. Should we make a new ticket for dictsort changes?

comment:6 Changed 5 years ago by Baptiste Mispelon

Seeing how far we've deviated from the original report, it might be better to open a new ticket with the correct details for this issue.

Can you do that and close this ticket when you've done so?

Thanks!

comment:7 Changed 5 years ago by Tim Graham

Resolution: duplicate
Status: newclosed

Done in #25670

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