Opened 4 years ago

Closed 4 years ago

#17888 closed Cleanup/optimization (fixed)

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

Reported by: brutasse 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 brutasse 4 years ago.
Patch that breaks the current regression tests

Download all attachments as: .zip

Change History (5)

Changed 4 years ago by brutasse

Patch that breaks the current regression tests

comment:1 Changed 4 years ago by jezdez

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Yes, and yes!

comment:2 Changed 4 years ago by brutasse

Pull request with tests and docs is at

comment:3 Changed 4 years ago by mjtamlyn

  • Cc marc.tamlyn@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready 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 set to fixed
  • Status changed from new to closed

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