fix: include Sentry API response body in provisioning errors

[c1-dev-bot]

Summary

• Include the Sentry API response body in error messages from AddMemberToOrganization and DeleteMemberFromOrganization, so 400/4xx errors are actually debuggable instead of just showing "unexpected status code: 400"
• Add readBody() helper that reads and returns the response body for inclusion in error messages (with 1024-byte truncation)
• Detect "member already exists" responses from the Sentry API and return a sentinel ErrMemberAlreadyExists error
• Handle ErrMemberAlreadyExists in CreateAccount to treat already-existing members as success rather than failing the provisioning ticket

Context

An issue was reported where account provisioning failed with 400 Bad Request but the error message contained no details about why Sentry rejected the request. The logBody() function logged the response body but did not include it in the returned error, making debugging impossible without access to connector logs.

Additionally, if a user is already a member of the Sentry organization, Sentry returns a 400 with "member already exists" / "already a member" in the response body. The connector now handles this gracefully by succeeding the account creation instead of failing.

Test plan

• Build passes (go build ./...)
• Existing tests pass (go test ./...)
• Manual verification: provisioning a new member returns success
• Manual verification: provisioning an already-existing member returns success instead of error
• Manual verification: other 400 errors (e.g., invalid role) now include the Sentry error details in the error message

Fixes: CXH-1175

---

Automated PR Notice

This PR was automatically created by c1-dev-bot as a potential implementation.

This code requires:

• Human review of the implementation approach
• Manual testing to verify correctness
• Approval from the appropriate team before merging

[linear]

CXH-1175 Sentry V2: Account provisioning error

[github-actions]

Claude finished @c1-dev-bot[bot]'s task in 1m 23s —— View job

---

PR Review: Include Sentry API response body in provisioning errors

• Read changed files and gather context
• Review code for correctness, security, and best practices
• Post review feedback

---

Overall: Good improvement — surfacing the API response body in errors is clearly the right call for debuggability, and handling the "already exists" case gracefully is a solid addition. A few issues to address:

Issues Found

1. Double-close of res.Body (pkg/client/organizations.go:148, also in DeleteMemberFromOrganization): readBody() closes the body via defer, but the caller then also runs defer res.Body.Close(). This is a double-close bug. Consider having readBody accept an io.Reader instead of io.ReadCloser so the caller retains close ownership, or restructure the control flow to return before the second defer.

2. Full body logged before truncation (pkg/client/helpers.go:40-44): readBody logs the entire body at Info level before truncating. This could produce very large log entries in production. Log after truncation, or use Debug level. See inline suggestion.

3. Error message formatting (pkg/client/organizations.go:142): The %w: %s format with the body appended after a colon can be confusing in error chains. A clearer separator like (response: %s) would improve readability. See inline suggestion.

Minor / Questions

4. CreateAccountResponse_ActionRequiredResult for already-exists (pkg/connector/users.go:123): This returns the same response type as the success path. Left an inline question about whether this is intentional or if a SuccessResult with the existing member reference would be more appropriate.

5. String-based error detection (isMemberAlreadyExistsError): Matching on substrings in the response body is fragile — if Sentry changes wording, this will silently break. Not a blocker since there's no better option with Sentry's API, but worth noting. Consider also checking for HTTP 409 status code if Sentry ever returns that for duplicates.

See inline comments for specific suggestions.

[github-actions]

nit: The full (potentially large) body is logged before truncation. Consider logging only the truncated string, or at least use Debug level instead of Info to avoid dumping large payloads into production logs.

Suggested change

	s := string(body)
	l.Info("response body", zap.String("body", s))
	if len(s) > 1024 {
	s = s[:1024]
	}
	s := string(body)
	if len(s) > 1024 {
	s = s[:1024]
	}
	l.Debug("response body", zap.String("body", s))

[github-actions]

Bug: When readBody is called here, it closes res.Body via defer bodyCloser.Close(). But execution then continues to line 148 (defer res.Body.Close()), which will attempt to close the already-closed body — a double-close. While this is unlikely to cause a runtime error with most HTTP implementations, it's technically incorrect.

Since readBody already closes the body, you should return immediately after the body-related error paths, or skip the defer res.Body.Close() when the body has already been consumed. The same pattern applies to DeleteMemberFromOrganization.

One clean approach: move the defer res.Body.Close() above the status check and have readBody accept io.Reader instead of io.ReadCloser (not closing it), so the caller retains ownership of closing.

[github-actions]

Bug: When the error path detects "member already exists", it returns ErrMemberAlreadyExists — a new sentinel error that does not wrap the original err. This means the original HTTP error context (status code, URL, etc.) is silently discarded. This is fine for the "already exists" case since the caller treats it as success, but worth noting.

More importantly, on line 142, the error format %w: %s produces something like "failed to add member to organization: unexpected status code 400: {"detail":"..."}". The %w wrapping is correct here, but the appended body text after the colon could be confusing since it looks like it might be part of the wrapped error chain. Consider using a clearer separator:

Suggested change

	if body != "" {
	return fmt.Errorf("failed to add member to organization: %w: %s", err, body)
	}
	if body != "" {
	return fmt.Errorf("failed to add member to organization: %w (response: %s)", err, body)
	}

[github-actions]

Question: CreateAccountResponse_ActionRequiredResult is also used on the success path (line 128). Is this the correct response type for the "already exists" case? Typically, per the CLAUDE.md guidance on grant idempotency, you'd want to signal "already exists" distinctly. However, since there's no CreateAccountResponse_SuccessResult with an existing resource reference being returned here either way, this seems acceptable — just want to confirm this is intentional and not a case where a SuccessResult with the existing member's resource would be more appropriate.