Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix http_client input connect bug #188

Merged
merged 1 commit into from
Mar 2, 2025

Conversation

mihaitodor
Copy link
Collaborator

@mihaitodor mihaitodor commented Mar 2, 2025

This issue was uncovered because the http_client components would not try to reconnect if the internal codec couldn't be initialised. The fix addresses the issue at the scanner level.

h.codecCtor.Create will return a *service.OwnedScanner, but h.codec has type codec.DeprecatedFallbackStream, so if we get an error from h.codecCtor.Create, the h.codec != nil check in Connect will be true, so Benthos will think the input is connected. This can happen if the codec specification isn't compatible with the received data.

Copy link
Collaborator

@rockwotj rockwotj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

This issue was uncovered because the `http_client` components
would not try to reconnect if the internal codec couldn't be
initialised. The fix addresses the issue at the scanner level.

`h.codecCtor.Create` will return a `*service.OwnedScanner`, but
`h.codec` has type `codec.DeprecatedFallbackStream`, so if we get
an error from `h.codecCtor.Create`, the `h.codec != nil` check in
`Connect` will be true, so Benthos will think the input is
connected. This can happen if the codec specification isn't
compatible with the received data.

Signed-off-by: Mihai Todor <[email protected]>
@mihaitodor mihaitodor force-pushed the mihaitodor-fix-http-client-input-connect-bug branch from 60d5253 to ba69e0f Compare March 2, 2025 15:42
@mihaitodor mihaitodor requested a review from rockwotj March 2, 2025 15:44
@mihaitodor
Copy link
Collaborator Author

mihaitodor commented Mar 2, 2025

I ended up refining the fix since any component which uses scanners through the old codec APIs can bump into this... I also added a test (and noticed that Testify's Nil assertion doesn't distinguish between nil interfaces and non-nil interfaces with nil underlying data 🙈).

@mihaitodor mihaitodor merged commit 7011b37 into main Mar 2, 2025
3 checks passed
@mihaitodor mihaitodor deleted the mihaitodor-fix-http-client-input-connect-bug branch March 2, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants