Cyclic dependencies

[elfjes]

Currently, cyclic dependencies raise a recursion error. It might be nice to obtain a more user friendly error message

def test_detects_circular_dependency_and_raises(registry, container):
    """
    If a factory causes a circular dependency lookup, a friendly error is raised
    showing the dependency cycle
    """

    def int_factory(svcs_container):
        svcs_container.get(str)
        return 42

    def str_factory(svcs_container):
        svcs_container.get(int)
        return "42"

    registry.register_factory(int, int_factory)
    registry.register_factory(str, str_factory)

    with pytest.raises(Exception) as e:
        container.get(int)

    assert 'cyclic dependency detected:  int -> str -> int'  in str(e.value)

[hynek]

This might be more complicated than it sounds – in attrs we had to use thread locals for recursive reprs. 🤔

[elfjes]

yes I see, concurrency makes this tricky, Also async factories complicate things. Nothing a contextvar can't fix, but I guess you'd want a contextvar per container.

If you want I could whip up a PR to discuss things. Although the effort/added complexity might outweigh the benefit for now :)

[hynek]

to be clear: I've skipped a step here. In svcs's case there's no hidden DAG, so get() would have to keep track of what it already got. So it could be global variable, where concurrency comes into play.

But as you say, this is complexity and this also would have an impact on performance. Given the SOP, it should only happen if you misconfigure something and you should be able to find the culprit quite quick.

---

but I'm happy to leave it open and wait what others have to say