`TypeError` when loading a Django app with incorrect type of `choices`

[Natalia Bidart]

While reviewing the PR for #31262, I was performing some local changes to ensure that the error messages shown by the check framework were accurate and correct, and I changed a choices definition to be just an int. This change made the reloader fail with an exception:

  File "/home/nessita/fellowship/projectfromrepo/ticket_31262/models.py", line 31, in <module>
    class OtherTestModelForm(forms.ModelForm):
  File "/home/nessita/fellowship/django/django/forms/models.py", line 308, in __new__
    fields = fields_for_model(
             ^^^^^^^^^^^^^^^^^
  File "/home/nessita/fellowship/django/django/forms/models.py", line 233, in fields_for_model
    formfield = f.formfield(**kwargs)
                ^^^^^^^^^^^^^^^^^^^^^
  File "/home/nessita/fellowship/django/django/db/models/fields/__init__.py", line 2384, in formfield
    return super().formfield(
           ^^^^^^^^^^^^^^^^^^
  File "/home/nessita/fellowship/django/django/db/models/fields/__init__.py", line 2136, in formfield
    return super().formfield(
           ^^^^^^^^^^^^^^^^^^
  File "/home/nessita/fellowship/django/django/db/models/fields/__init__.py", line 1112, in formfield
    defaults["choices"] = self.get_choices(include_blank=include_blank)
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nessita/fellowship/django/django/db/models/fields/__init__.py", line 1048, in get_choices
    choices = list(self.choices)
              ^^^^^^^^^^^^^^^^^^
TypeError: 'int' object is not iterable

I believe that the code in Field.get_choices could be a little more robust, so exceptions are not raised on incorrect choices type. Ideally we'd let the check framework run and report these issues is a friendlier manner.

[Mariusz Felisiak]

I believe that the code in Field.get_choices could be a little more robust, so exceptions are not raised on incorrect choices type.

I'm not sure it's worth changing, it's clearly user error that's easy to notice, understand, and fix. Furthermore, there are other places that assume choices is iterable (e.g. flatchoices). Are we going to add protection to all of them? You can find many such edge cases in Django source code, when a type is assumed. IMO, we don't need to be overprotective. Especially since this is already covered by a system check.

Ideally we'd let the check framework run and report these issues is a friendlier manner.

Autoreloader cannot run checks after each change, this would have bad impact on developers experience. We even added an option to not run checks even once, check out #32296.

[Natalia Bidart]

Replying to Mariusz Felisiak:

I believe that the code in Field.get_choices could be a little more robust, so exceptions are not raised on incorrect choices type.

I'm not sure it's worth changing, it's clearly user error that's easy to notice, understand, and fix. Furthermore, there are other places that assume choices is iterable (e.g. flatchoices). Are we going to add protection to all of them? You can find many such edge cases in Django source code, when a type is assumed. IMO, we don't need to be overprotective. Especially since this is already covered by a system check.

Fair enough!

Ideally we'd let the check framework run and report these issues is a friendlier manner.

Right, my point is that the check framework never reports this issue (not even on first start) because Django crashes before the checks are run.

Autoreloader cannot run checks after each change, this would have bad impact on developers experience. We even added an option to not run checks even once, check out #32296.

Thanks for the pointer, though note that it seems that system checks are run on every reload (if no special option is passed):

(djangodev) [nessita@socrates projectfromrepo]$ python -Wall manage.py runserver 0.0.0.0:9000
Watching for file changes with StatReloader
Performing system checks...

System check identified no issues (0 silenced).
July 21, 2023 - 13:36:06
Django version 5.0.dev20230720170441, using settings 'projectfromrepo.settings'
Starting development server at http://0.0.0.0:9000/
Quit the server with CONTROL-C.

/home/nessita/fellowship/projectfromrepo/ticket_34711/models.py changed, reloading.
Watching for file changes with StatReloader
Performing system checks...

System check identified no issues (0 silenced).
July 21, 2023 - 13:36:22
Django version 5.0.dev20230720170441, using settings 'projectfromrepo.settings'
Starting development server at http://0.0.0.0:9000/
Quit the server with CONTROL-C.

/home/nessita/fellowship/projectfromrepo/ticket_34711/models.py changed, reloading.
Watching for file changes with StatReloader
Performing system checks...

Exception in thread django-main-thread:
Traceback (most recent call last):
  File "/usr/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.11/threading.py", line 975, in run
    self._target(*self._args, **self._kwargs)
  File "/home/nessita/fellowship/django/django/utils/autoreload.py", line 64, in wrapper
    fn(*args, **kwargs)
  File "/home/nessita/fellowship/django/django/core/management/commands/runserver.py", line 133, in inner_run
    self.check(display_num_errors=True)
  File "/home/nessita/fellowship/django/django/core/management/base.py", line 556, in check
    raise SystemCheckError(msg)
django.core.management.base.SystemCheckError: SystemCheckError: System check identified some issues:

ERRORS:
ticket_34711.OtherTestModel.asset2: (fields.E009) 'max_length' is too small to fit the longest value in 'choices' (4 characters).

System check identified 1 issue (0 silenced).

See all the Performing system checks when a file is changed. I'm not arguing the ticket resolution, I just thought it'd be nice to not have Django crashing before even running the checks, but I agree the reported crash is clear and easily fixable.