Django.db.models.deletion related_objects takes 3 positional arguments

Question

I'm upgrading my project from Django 2.2 to 3.2 and wracking my brain at what seems to be a bug in their code.

I have a test that does a simple DELETE request to a resource (incidentally a DjangoRestFramework resource, DRF version is 3.12.4), and a crash happens inside django.db.models.deletion. here is the relevant part of the stack trace:

        response = admin_client.delete(
            reverse(self.url_name, args=[project.pk, category.pk]),
>           content_type='application/json'
        )

test_category_views.py:609: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/home/rn/venvs/lib/python3.6/site-packages/django/test/client.py:795: in delete
    response = super().delete(path, data=data, content_type=content_type, secure=secure, **extra)
/home/rn/venvs/lib/python3.6/site-packages/django/test/client.py:447: in delete
    secure=secure, **extra)
/home/rn/venvs/lib/python3.6/site-packages/django/test/client.py:473: in generic
    return self.request(**r)
/home/rn/venvs/lib/python3.6/site-packages/django/test/client.py:719: in request
    self.check_exception(response)
/home/rn/venvs/lib/python3.6/site-packages/django/test/client.py:580: in check_exception
    raise exc_value
/home/rn/venvs/lib/python3.6/site-packages/django/core/handlers/exception.py:47: in inner
    response = get_response(request)
../../../hazardlog/platform.py:127: in _get_response
    response = self.process_exception_by_middleware(e, request)
../../../hazardlog/platform.py:125: in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
/usr/local/lib/python3.6/contextlib.py:52: in inner
    return func(*args, **kwds)
/home/rn/venvs/lib/python3.6/site-packages/django/views/decorators/csrf.py:54: in wrapped_view
    return view_func(*args, **kwargs)
/home/rn/venvs/lib/python3.6/site-packages/django/views/generic/base.py:70: in view
    return self.dispatch(request, *args, **kwargs)
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/views.py:509: in dispatch
    response = self.handle_exception(exc)
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/views.py:469: in handle_exception
    self.raise_uncaught_exception(exc)
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/views.py:480: in raise_uncaught_exception
    raise exc
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/views.py:506: in dispatch
    response = handler(request, *args, **kwargs)
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/generics.py:291: in delete
    return self.destroy(request, *args, **kwargs)
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/mixins.py:91: in destroy
    self.perform_destroy(instance)
/home/rn/venvs/lib/python3.6/site-packages/rest_framework/mixins.py:95: in perform_destroy
    instance.delete()
/home/rn/venvs/lib/python3.6/site-packages/django/db/models/base.py:953: in delete
    collector.collect([self], keep_parents=keep_parents)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <django.db.models.deletion.Collector object at 0x7f7870265ac8>
objs = [<Category: Test category 0>], source = None, nullable = False
collect_related = True, source_attr = None, reverse_dependency = False
keep_parents = False, fail_on_restricted = True

    def collect(self, objs, source=None, nullable=False, collect_related=True,
                source_attr=None, reverse_dependency=False, keep_parents=False,
                fail_on_restricted=True):
        """
        Add 'objs' to the collection of objects to be deleted as well as all
        parent instances.  'objs' must be a homogeneous iterable collection of
        model instances (e.g. a QuerySet).  If 'collect_related' is True,
        related objects will be handled by their respective on_delete handler.
    
        If the call is the result of a cascade, 'source' should be the model
        that caused it and 'nullable' should be set to True, if the relation
        can be null.
    
        If 'reverse_dependency' is True, 'source' will be deleted before the
        current model, rather than after. (Needed for cascading to parent
        models, the one case in which the cascade follows the forwards
        direction of an FK rather than the reverse direction.)
    
        If 'keep_parents' is True, data of parent model's will be not deleted.
    
        If 'fail_on_restricted' is False, error won't be raised even if it's
        prohibited to delete such objects due to RESTRICT, that defers
        restricted object checking in recursive calls where the top-level call
        may need to collect more objects to determine whether restricted ones
        can be deleted.
        """
        if self.can_fast_delete(objs):
            self.fast_deletes.append(objs)
            return
        new_objs = self.add(objs, source, nullable,
                            reverse_dependency=reverse_dependency)
        if not new_objs:
            return
    
        model = new_objs[0].__class__
    
        if not keep_parents:
            # Recursively collect concrete model's parent models, but not their
            # related objects. These will be found by meta.get_fields()
            concrete_model = model._meta.concrete_model
            for ptr in concrete_model._meta.parents.values():
                if ptr:
                    parent_objs = [getattr(obj, ptr.name) for obj in new_objs]
                    self.collect(parent_objs, source=model,
                                 source_attr=ptr.remote_field.related_name,
                                 collect_related=False,
                                 reverse_dependency=True,
                                 fail_on_restricted=False)
        if not collect_related:
            return
    
        if keep_parents:
            parents = set(model._meta.get_parent_list())
        model_fast_deletes = defaultdict(list)
        protected_objects = defaultdict(list)
        for related in get_candidate_relations_to_delete(model._meta):
            # Preserve parent reverse relationships if keep_parents=True.
            if keep_parents and related.model in parents:
                continue
            field = related.field
            if field.remote_field.on_delete == DO_NOTHING:
                continue
            related_model = related.related_model
            if self.can_fast_delete(related_model, from_field=field):
                model_fast_deletes[related_model].append(field)
                continue
            batches = self.get_del_batches(new_objs, [field])
            for batch in batches:
>               sub_objs = self.related_objects(related_model, [field], batch)
E               TypeError: related_objects() takes 3 positional arguments but 4 were given

/home/rn/venvs/ramrisk/lib/python3.6/site-packages/django/db/models/deletion.py:282: TypeError

So, related_objects is called with 4 positional args even though it supposedly only accepts 3. Python counts self with these, so the 4 is correct. It gets called with self, related_model, [field], and batch. Great. So, let's look at the definition of self.related_objects in django.db.models.deletion:

    def related_objects(self, related_model, related_fields, objs):
        """
        Get a QuerySet of the related model to objs via related fields.
        """
        predicate = reduce(operator.or_, (
            query_utils.Q(**{'%s__in' % related_field.name: objs})
            for related_field in related_fields
        ))
        return related_model._base_manager.using(self.using).filter(predicate)

Right, well, that quite clearly takes 4 parameters as I'd expect, so where does that TypeError come from? Keep in mind, I've only shown you Django 3.2 code and none of my own. Even if I somehow put something crazy into these variables, they should still never be able to produce that error...

Answer 1

Alright, found my answer. Actually this is probably something no-one would have been able to guess, but I just want to share what I learned.

So I was right, the error does not make sense, because it doesn't fit with the function signature. It should never be able to happen. So how to debug that?

Well, my first instinct was that maybe this function definition is replaced at runtime somewhere. How would I check that? I set a breakpoint at the line that calls the method, and then in my debugger, I do

import inspect
inspect.getmodule(self.related_objects)

This gave me my answer. Seems like we've monkey-patched that function to decorate it with extra functionality, but now that the Django version is upgraded, the expected signature is changed.

This is exactly why Monkey patching is dangerous and not to be used carelessly. Lesson here: If you do monkey patching, be sure to revisit all of those whenever you upgrade the lib versions. This one failed, so I had to find out what was happening, but it could easily have been more sinister. It could have silently done something different and incorrect, because of small changes to the lib, which the monkey patch was unaware of.