Fix/informative error message

[RafaelJohn9]

Related Issues

• fixes Return a more informative error message when trying to connect two components when the sender component is missing it's OutputSockets #9570

Proposed Changes:

Made the error more descriptive using the formatted recommended by @sjrl .

How did you test it?

• manual verification

Notes for the reviewer

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added unit tests and updated the docstrings
• I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
• I documented my code
• I ran pre-commit hooks and fixed any issue

[coveralls]

Pull Request Test Coverage Report for Build 16902023987

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 50 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.001%) to 91.97%

---

Files with Coverage Reduction	New Missed Lines	%
core/pipeline/base.py	50	86.92%

Totals
Change from base Build 16880323123:	0.001%
Covered Lines:	12840
Relevant Lines:	13961

---

💛 - Coveralls

[RafaelJohn9]

Hey @sjrl ,

On the first recommended error message when sender_sockets is not empty.

haystack.core.errors.PipelineConnectError: 'valid_component.result does not exist. Output connections of valid_component are:

do you think this would be a more clear error message:

haystack.core.errors.PipelineConnectError: Components 'comp1' and 'comp2' are already connected. 'comp1.value' is already connected to 'comp2.value'

Reason

I was debugging the test : test/core/pipeline/test_pipeline_base.py::TestPipelineBase::test_connect_already_connected
which now fails with the new logic included.

With the current error message this is what it looks like

FAILED test/core/pipeline/test_pipeline_base.py::TestPipelineBase::test_connect_already_connected - haystack.core.errors.PipelineConnectError: 'comp1.value' does not exist. Output connections of comp1 are: value (type int)

and I thought this would add more clarity

FAILED test/core/pipeline/test_pipeline_base.py::TestPipelineBase::test_connect_already_connected - haystack.core.errors.PipelineConnectError: Components 'comp1' and 'comp2' are already connected. 'comp1.value' is already connected to 'comp2.value'.

Would love to hear your thoughts on this

[sjrl]

and I thought this would add more clarity

FAILED test/core/pipeline/test_pipeline_base.py::TestPipelineBase::test_connect_already_connected - haystack.core.errors.PipelineConnectError: Components 'comp1' and 'comp2' are already connected. 'comp1.value' is already connected to 'comp2.value'.

Would love to hear your thoughts on this

Great catch! Yes that would be a much better error message to provide. So let's go with your suggestion for this third case.

[RafaelJohn9]

I do apologize for disabling C901 complexity check inside PipelineBase.connect method, though I had the following precommit error.

haystack/core/pipeline/base.py:434:9: C901 connect is too complex (21 > 20)    |432 |    

[RafaelJohn9]

hey @sjrl ,

upon further review, I would recommend not adding the if not receiver_sockets: for now.

When we create a component with no input sockets

def test_connect_with_receiver_component_with_no_input_sockets():
        """
        Test connecting to a component that has no input sockets.
        Should raise PipelineConnectError.
        """

        @component
        class NoInputComponent:
            @component.output_types(result=int)
            def run(self, result: int):
                return {"result": result}

        comp1 = component_class("Comp1", output_types={"value": int})()
        comp2 = NoInputComponent()
        pipe = PipelineBase()
        pipe.add_component("comp1", comp1)
        pipe.add_component("comp2", comp2)

We get a TypeError since we are trying to pass in an arg for a component that doesn't have any params

================================================================================== short test summary info ===================================================================================
FAILED test/core/pipeline/test_pipeline_base.py::TestPipelineBase::test_connect_with_receiver_component_with_no_input_sockets - TypeError: TestPipelineBase.test_connect_with_receiver_component_with_no_input_sockets() takes 0 positional arguments but 1 was given

this means if we add the if not receiver_sockets: statement, it may end up as deadcode.

Though adding a test to assert this behaviour(TypeError) remains the same, would help.

Would love to hear your thoughts.

[sjrl]

Hmm I think the test is failing with this TypeError because the self is missing as the positional argument in the function definition. So the below line needs to be

def test_connect_with_receiver_component_with_no_input_sockets(self):

However, thinking more on this I don't think this check makes sense since input types of components are automatically set based on the input signature of the run method so theoretically receiver_sockets should never be empty. So I also think it's fine to not add this check.

[sjrl]

Thanks for your work on this!