fix memory leak

[adiom-mark]

Summary by CodeRabbit

• Bug Fixes
  • Improved connector error handling to ensure proper resource cleanup when operation failures occur, enhancing system stability and preventing resource leaks.

[coderabbitai]

Warning

Rate limit exceeded

@adiom-mark has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 43 minutes and 44 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 44 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d250b222-8468-4ec3-b6c9-4dcc44dca193

📥 Commits

Reviewing files that changed from the base of the PR and between 33e3b3d and e2e8882.

📒 Files selected for processing (4)

• connectors/kafka/kafkawrap.go
• connectors/mongo/conn.go
• connectors/testconn/connector.go
• internal/app/options/connectorflags.go

📝 Walkthrough

Walkthrough

A teardownable interface and teardownConnector helper function were added to enable resource cleanup. The ConfigureConnectors and KafkaWrap functions now call teardown on failure paths to properly release connector resources when setup fails.

Changes

Cohort / File(s)	Summary
Connector Teardown internal/app/options/connectorflags.go	Added teardownable interface with Teardown() method and teardownConnector() helper to invoke teardown on Local/Remote connector components. Updated ConfigureConnectors() to call teardown when destination is missing, creation fails, help commands error, or destination is unsupported. Updated KafkaWrap() to call teardown on inner connector missing, creation failures, and wrapping failures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A cleanup dance for connectors bright,
When teardowns fail, we make things right,
No resources left behind to roam,
Each connector finds a tidy home! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix memory leak' is vague and generic—it does not specify which memory leak is being fixed or what component is affected, making it unclear to reviewers scanning history.	Provide a more specific title that identifies the affected component, such as 'Fix memory leak in connector teardown' or 'Fix connector resource cleanup on failure paths'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fixleak

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/app/options/connectorflags.go (1)

578-611: ⚠️ Potential issue | 🟡 Minor

Missing error handling when no registered connector matches.

If the loop completes without finding a matching connector for restArgs[0], execution falls through to line 611 returning err (which is nil at this point). This causes the unwrapped conn to be returned (lines 617-618) instead of an error, which is incorrect for kafka-wrap.

Note: The teardown calls in the loop (lines 573, 583, 594, 603) are not actually effective since kafkaConn does not implement the teardownable interface—the type assertions will always fail. The primary issue is the missing error return.

Proposed fix

 			}
 		}
 	}
 
+	if t, ok := conn.(teardownable); ok {
+		t.Teardown()
+	}
+	return fmt.Errorf("no matching connector found for kafka-wrap: %s", restArgs[0])
-	return err
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/app/options/connectorflags.go` around lines 578 - 611, The loop over
registeredConnectors can complete without finding a match and currently falls
through returning a nil err, allowing an unwrapped conn to be used; update the
code after the loop to return a descriptive error (e.g., fmt.Errorf("no
registered connector for %q", restArgs[0])) when no
connector.IsConnector(restArgs[0]) was found, and ensure any necessary teardown
is invoked on conn if it implements teardownable (use the same t, ok type
assertion) before returning the error; locate the loop that iterates
registeredConnectors and references restArgs, conn, teardownable and
kafka.NewKafkaWrapConn to apply this change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@internal/app/options/connectorflags.go`:
- Around line 578-611: The loop over registeredConnectors can complete without
finding a match and currently falls through returning a nil err, allowing an
unwrapped conn to be used; update the code after the loop to return a
descriptive error (e.g., fmt.Errorf("no registered connector for %q",
restArgs[0])) when no connector.IsConnector(restArgs[0]) was found, and ensure
any necessary teardown is invoked on conn if it implements teardownable (use the
same t, ok type assertion) before returning the error; locate the loop that
iterates registeredConnectors and references restArgs, conn, teardownable and
kafka.NewKafkaWrapConn to apply this change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a5c227f0-551f-4f80-b982-868eaea2b826

📥 Commits

Reviewing files that changed from the base of the PR and between f246f8a and 33e3b3d.

📒 Files selected for processing (1)

• internal/app/options/connectorflags.go