fix: remove content on unmarshal error

[Dav-14]

No description provided.

[coderabbitai]

Walkthrough

The changes refactor the payments connector configuration builder by renaming CreateConfig() to installConfig(ctx), adding context-aware error logging for unmarshalling and marshalling failures, and reorganizing the test file from external to internal package scope for direct access to unexported types and methods.

Changes

Cohort / File(s)	Summary
Main Implementation internal/resources/payments_connectors.go	Renamed CreateConfig() method to installConfig(ctx context.Context) with added context parameter. Integrated context-aware error logging via logging.FromContext(ctx) on unmarshalling, marshalling, and parsing failures. Updated all call sites in Create and Update flows to use new signature.
Test File Reorganization internal/resources/payments_connectors_test.go	Moved test package from resources_test to resources to enable direct access to unexported identifiers. Updated all type and method references from resources.* qualified names to local scope (e.g., resources.PaymentsConnectorsModel → PaymentsConnectorsModel, plan.CreateConfig() → plan.installConfig(logging.TestingContext())). Added new test case TestPaymentsCreateConfigFromModel_Error to validate error handling for invalid config unmarshalling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify all call sites have been updated to pass context parameter to installConfig(ctx)
• Confirm error logging statements use logging.FromContext(ctx) correctly
• Validate new error handling test case (TestPaymentsCreateConfigFromModel_Error) covers edge cases
• Ensure context propagation is correct throughout Create and Update flows

Poem

🐰 A context hops through the code today,
Error logs now know the hoppy way,
The tests moved in to play,
Unexported methods dance and sway,
Refactoring makes the flow okay! 🌟

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset.	Add a description explaining the motivation, implementation details, and any relevant context for the changes made to error handling and logging.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: remove content on unmarshal error' is specific and relates to the main change of refactoring error handling to remove sensitive content from error messages during unmarshalling failures.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/error_includes_api_key

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/resources/payments_connectors_test.go (1)

105-119: Excellent test coverage for security fix!

This test validates that sensitive data (API key) is not leaked in error messages, which directly supports the PR objective.

Consider adding a test case that triggers the credentials unmarshal error path (line 65-67 in payments_connectors.go) to ensure comprehensive coverage of all error paths:

func TestPaymentsCreateConfigFromModel_CredentialsError(t *testing.T) {
	t.Parallel()

	plan := PaymentsConnectorsModel{
		Credentials: types.DynamicValue(types.StringNull()),
		Config: types.DynamicValue(NewDynamicObjectValue(map[string]attr.Value{
			"provider": types.DynamicValue(types.StringValue("Generic")),
		}).Value()),
	}

	_, err := plan.installConfig(logging.TestingContext())
	require.Error(t, err)
	require.Contains(t, err.Error(), "failed to unmarshal credentials")
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b145b2d and 1a0a27e.

📒 Files selected for processing (2)

• internal/resources/payments_connectors.go (5 hunks)
• internal/resources/payments_connectors_test.go (9 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: Dav-14
Repo: formancehq/terraform-provider-stack PR: 30
File: internal/resources/payments_connectors.go:75-83
Timestamp: 2025-10-13T11:09:25.968Z
Learning: In the formancehq/terraform-provider-stack repository, prefer API-side validation over client-side validation in the Terraform provider for payment connector fields like provider, rather than adding validation checks in the Terraform provider code.

🧬 Code graph analysis (1)

internal/resources/payments_connectors_test.go (2)

internal/resources/payments_connectors.go (3)

• PaymentsConnectorsModel (53-57)
• ExtractKeys (22-30)
• SanitizeUnknownKeys (32-41)

internal/resources/helpers.go (1)

• NewDynamicObjectValue (16-20)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Tests (E2E)

🔇 Additional comments (5)

internal/resources/payments_connectors.go (3)

59-59: LGTM! Method signature improved.

The rename to installConfig with context parameter enables proper logging while reducing the public API surface by making the method unexported.

---

65-91: Good security-focused error handling.

The error handling for credentials unmarshal (lines 65-67), marshal (lines 70-73), and config unmarshal (lines 88-90) correctly logs detailed errors to context while returning generic messages. This prevents sensitive data leakage.

---

222-222: LGTM! Call sites properly updated.

Both Create and Update methods correctly invoke installConfig(ctx) with the context parameter.

Also applies to: 325-325

internal/resources/payments_connectors_test.go (2)

1-1: LGTM! Package scope change enables internal testing.

Moving from resources_test to resources allows direct testing of the now-unexported installConfig method. This is appropriate for white-box testing of internal behavior.

---

18-103: LGTM! Tests properly updated for API changes.

The existing tests correctly use the new installConfig(logging.TestingContext()) signature and adjust type references for the internal package scope.

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.04%. Comparing base (1fe11c8) to head (1a0a27e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/resources/payments_connectors.go	33.33%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   55.16%   58.04%   +2.87%     
==========================================
  Files          20       20              
  Lines        1519     1311     -208     
==========================================
- Hits          838      761      -77     
+ Misses        566      437     -129     
+ Partials      115      113       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.