TPT-3483: Update NewClient method to return an error rather than just logging errors

[zliang-akamai]

📝 Description

Intentional breaking changes to various client related functions to return the error if encountered rather than only logging it.

✔️ How to Test

make test-int

[Copilot]

Pull request overview

This PR introduces intentional breaking changes to the client constructors so that client initialization failures are returned to the caller instead of being only logged, improving error visibility and allowing callers/tests to fail fast.

Changes:

• Updated NewClient to return (Client, error) and propagated error handling to NewClientFromEnv and test/client creation helpers.
• Changed SetRootCertificate on both Client and MonitorClient to return error instead of chaining, and updated related tests.
• Added/updated test utilities (CreateMockClientWithError, newTestClient) to support constructors that can fail.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
client.go	NewClient now returns an error; SetRootCertificate returns error and is used during initialization.
client_monitor.go	MonitorClient.SetRootCertificate now returns error and conditionally logs.
internal/testutil/mock.go	Adds CreateMockClientWithError to support error-returning constructors in tests.
client_test.go	Updates tests for new constructor/error semantics; adds newTestClient helper and new root CA error-path tests.
config_test.go	Updates to use newTestClient instead of NewClient(nil) directly.
errors_test.go	Updates test helper to use newTestClient and marks it as a test helper.
request_helpers_test.go	Updates mock client creation to use CreateMockClientWithError.
test/integration/integration_suite_test.go	Handles NewClient error return in integration test client setup.
test/unit/base.go	Updates unit-test base client creation to use CreateMockClientWithError.
test/unit/util_test.go	Updates mock client creation helper to use CreateMockClientWithError.

Comments suppressed due to low confidence (2)

client.go:309

• AppendCertsFromPEM returns a bool indicating whether any certificates were successfully parsed and added. Right now its return value is ignored, so invalid/empty PEM files will be reported as success and the client will silently proceed without the intended CA trust. Consider checking the boolean and returning a descriptive error when no certs could be appended.

	pem, err := os.ReadFile(filepath.Clean(certPath))
	if err != nil {
		return fmt.Errorf("failed to read root certificate at %s: %w", certPath, err)
	}

	config.RootCAs.AppendCertsFromPEM(pem)

	return nil

client_monitor.go:164

• Like Client.SetRootCertificate, this ignores the boolean result of AppendCertsFromPEM, so invalid/empty PEM data will return nil error even though no CA was added. Consider checking the return value and returning an error if parsing/appending fails.

	pem, err := os.ReadFile(filepath.Clean(certPath))
	if err != nil {
		if mc.logger != nil {
			mc.logger.Errorf("Failed to read root certificate at %s: %s", certPath, err.Error())
		}

		return fmt.Errorf("failed to read root certificate at %s: %w", certPath, err)
	}

	transport.TLSClientConfig.RootCAs.AppendCertsFromPEM(pem)

	return nil

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ezilber-akamai]

Nice work!

[lgarber-akamai]

@zliang-akamai This looks great! Would you mind also updating the samples in the README accordingly?

[lgarber-akamai]

Great work!