Opened 5 years ago

Closed 4 years ago

#17888 closed Cleanup/optimization (fixed)

CheckboxInput.render() shouldn't catch exceptions from check_test

Reported by: Bruno Renié Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: marc.tamlyn@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


As the docs state, check_test is a callable that returns True if the checkbox should be checked for a specific value. The default implementation doesn't do anything complicated and shouldn't raise exceptions. If user-provided callables break, it django shouldn't catch the error silently.

My proposal is to simply remove that bare except and let check_test raise exceptions.

The current behaviour is actually expected and tested:

I still think it's quite a bad API and should be changed. Since that exception-swallowing behaviour isn't documented (although tested), backwards compatibility isn't that much of a concern.

Attachments (1)

17888.patch (652 bytes) - added by Bruno Renié 5 years ago.
Patch that breaks the current regression tests

Download all attachments as: .zip

Change History (5)

Changed 5 years ago by Bruno Renié

Attachment: 17888.patch added

Patch that breaks the current regression tests

comment:1 Changed 4 years ago by Jannis Leidel

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Yes, and yes!

comment:2 Changed 4 years ago by Bruno Renié

Pull request with tests and docs is at

comment:3 Changed 4 years ago by Marc Tamlyn

Cc: marc.tamlyn@… added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

There is an updated pull request at I think this is all good to go so I'm going to mark it as RFC.

comment:4 Changed 4 years ago by Alex Gaynor <alex.gaynor@…>

Resolution: fixed
Status: newclosed

In [6a5a12ea3e3193b3658b9237b86d98bc37d668d7]:

Fixed #17888 -- no longer silence exceptions inside of check_test. Thanks to brutasse for the patch.

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