Add detailed logging for GitHub API errors

[c1-dev-bot]

Summary

When a GitHub App returns "resource not accessible by integration" or other permission/auth errors, the connector's logs didn't include enough detail to identify which specific API call was failing. This made it very difficult to debug issues for customers using GitHub Enterprise with read-only permissions.

This PR adds structured logging for all GitHub API errors so that the exact failing endpoint, HTTP status, and GitHub error message are captured in logs.

Changes

• helpers.go: Added three new functions:

  • gitHubErrorMessage() - Extracts the full error message from *github.ErrorResponse including sub-error details (resource, field, code)
  • logGitHubAPIError() - Logs GitHub API errors at warn level with structured fields: operation, HTTP status, method, URL, and the GitHub error message
  • wrapGitHubErrorWithContext() - Combines logging + error wrapping for use in sync operations where context.Context is available
  • Enhanced wrapGitHubError() to include the GitHub error message in permission/auth error gRPC status messages

• All resource sync files (repository.go, org.go, team.go, user.go, org_role.go, api_token.go, invitation.go, enterprise_role.go, connector.go):

  • Updated error handling to use wrapGitHubErrorWithContext() for context-aware logging
  • Added specific resource identifiers (org name, repo name, team ID, role ID) to error messages
  • Enhanced existing 403-skip warn logs to include gitHubErrorMessage(err) so that silently-skipped resources show the exact GitHub error

Test plan

• go build ./... passes
• go test ./... passes
• Manual verification: configure a GitHub App with limited permissions and verify that logs now show which API endpoint returned "resource not accessible by integration"

Fixes: CXH-1226

---

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-1226 Add more logging to GitHub Connector

[github-actions]

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

---

PR Review: Add detailed logging for GitHub API errors

• Read and analyze all changed files
• Review helper functions in helpers.go
• Review changes across resource sync files
• Post review findings

---

Overall Assessment

This is a solid, well-scoped PR that addresses a real debugging pain point. The approach is clean — adding a wrapGitHubErrorWithContext layer that logs before wrapping, and enriching existing skip-on-403 warn logs with the actual GitHub error message. The changes are consistent across all resource sync files.

Approve with minor suggestions — see inline comments for details.

What looks good

• gitHubErrorMessage() — Good extraction of structured error details from github.ErrorResponse, including sub-errors (resource/field/code). The fallback to err.Error() for non-GitHub errors is sensible.
• logGitHubAPIError() — Structured logging with method, URL, and status code will make log searching much easier.
• wrapGitHubErrorWithContext() — Clean composition: log + delegate to existing wrapGitHubError. The nil check on err is correct.
• Resource identifiers in error messages — Adding org name, repo name, team ID, role ID to context messages (e.g., "failed to list teams for org %s") is exactly what's needed for multi-org debugging.
• Enhanced 403-skip logs — The existing l.Warn(...) calls that silently skip resources now include gitHubErrorMessage(err), which is the highest-value change in this PR.

Suggestions (minor, non-blocking)

1. Missing zap.Error(err) in logGitHubAPIError — The structured log only includes the parsed github_error string but not the full error chain. Adding zap.Error(err) would preserve transport-level context. See inline comment.

2. Redundant error fields in user.go:171-175 — Both zap.Error(err) and zap.String("github_error", ...) are present, producing overlapping info. See inline comment.

3. Rate-limit error messages don't get enriched — wrapGitHubError returns early for rate-limit errors without including contextMsg or the GitHub error message. The log from wrapGitHubErrorWithContext captures it, but the returned error doesn't. See inline comment.

No issues found with

• Error wrapping uses %w consistently
• No error swallowing — errors are either propagated or intentionally skipped with a warn log
• No nil pointer risks — resp is checked before access in all paths
• Prefix consistency — uses github-connector: or baton-github: matching existing patterns in each file

[github-actions]

Nit: For rate-limit and temporary-unavailability errors, wrapGitHubError returns early (lines 372, 378, 383, 388) without including the GitHub error message or contextMsg. This means wrapGitHubErrorWithContext will log the error at warn level via logGitHubAPIError, but the returned error won't contain the operation context or GitHub message — only generic strings like "rate limit exceeded" or "too many requests".

This is probably acceptable since rate-limit errors are transient and retried, but it's worth noting the asymmetry: auth/permission errors get the enriched message while rate-limit errors don't.

[github-actions]

Minor: zap.Error(err) and zap.String("github_error", gitHubErrorMessage(err)) will produce overlapping information. The Error(err) field will contain the full github.ErrorResponse.Error() string, and gitHubErrorMessage(err) extracts a subset of it. Not a bug, but slightly redundant. Consider keeping just one — either zap.Error(err) for the full error chain or zap.String("github_error", ...) for the parsed message.

[github-actions]

Consider also including zap.Error(err) in the fields list. Currently the log only includes the parsed github_error string, which loses the full error chain (e.g., any wrapped context from the HTTP transport layer). Adding the original error preserves the full chain for debugging:

Suggested change

	fields := []zap.Field{
	zap.String("operation", contextMsg),
	zap.String("github_error", gitHubErrorMessage(err)),
	}
	fields := []zap.Field{
	zap.String("operation", contextMsg),
	zap.Error(err),
	zap.String("github_error", gitHubErrorMessage(err)),
	}

[github-actions]

Claude finished @johnallers's task in 1m 22s —— View job

---

PR Review: Add detailed logging for GitHub API errors

• Read and analyze all changed files
• Review helper functions in helpers.go
• Review changes across resource sync files
• Check for issues flagged in previous review
• Post review findings

---

Overall Assessment

Approve with minor suggestions. This is a well-scoped PR that solves a real debugging pain point. The approach — adding wrapGitHubErrorWithContext as a log-then-wrap layer, enriching auth/permission error messages with gitHubErrorMessage(), and adding resource identifiers to all error contexts — is clean and consistently applied.

The second commit (62d414b) appropriately downgrades skip-path logs from Warn to Debug in org_role.go and repository.go, since these fire per-resource for limited-permission installations and would otherwise create excessive noise.

---

What looks good

• gitHubErrorMessage() — Clean extraction of structured details from github.ErrorResponse, including sub-errors. The err.Error() fallback for non-GitHub errors is correct.
• wrapGitHubErrorWithContext() — Good composition: nil check → log → delegate. This ensures every API error is captured with structured fields without changing existing error-wrapping semantics.
• Auth/permission error enrichment (helpers.go:392-395) — Including gitHubErrorMessage(err) in the gRPC status message for 401/403 errors is the highest-value change for customer-facing debugging.
• Resource identifiers in context — Adding org name, repo name, team ID, role ID to error messages (e.g., "failed to list teams for org %s") is exactly what's needed for multi-org debugging.
• Consistent application — All 10 files follow the same pattern; no missed call sites.

---

Issues to address

1. Missing zap.Error(err) in logGitHubAPIError (flagged in prior review)

helpers.go:340-343 — The structured log only includes the parsed github_error string but not the full error chain. Transport-level details (timeouts, TLS errors, wrapped context) are lost. This was flagged in the previous review and should be addressed:

fields := []zap.Field{
    zap.String("operation", contextMsg),
    zap.Error(err),  // preserves full error chain
    zap.String("github_error", gitHubErrorMessage(err)),
}

Fix this →

2. Redundant error fields in user.go:171-175 (flagged in prior review)

zap.Error(err) and zap.String("github_error", gitHubErrorMessage(err)) produce overlapping information — Error(err) contains the full github.ErrorResponse.Error() string, and gitHubErrorMessage() extracts a subset of it. Since this is a skip-and-continue path (not fed into logGitHubAPIError), keeping just zap.Error(err) is sufficient and less noisy:

l.Warn("error fetching user by id",
    zap.Error(err),
    zap.Int64("user_id", user.GetID()),
)

Alternatively, if you prefer the parsed format, keep only zap.String("github_error", ...) and drop zap.Error(err).

Fix this →

---

Minor observations (non-blocking)

• Log level inconsistency in org.go:128 — The "insufficient access to list org membership" skip path remains at Warn level while equivalent paths in org_role.go and repository.go were downgraded to Debug in the second commit. If the same reasoning applies (expected for limited-scope installs, fires per-org), it should also be Debug. Worth confirming intent.

• Rate-limit error messages not enriched — As noted in the prior review, wrapGitHubError returns early for rate-limit/abuse errors without including contextMsg or the GitHub error message. The wrapGitHubErrorWithContext log captures it, so the info isn't lost — but the returned error won't carry the operation context. Acceptable as-is since these are transient and retried.

• gitHubErrorMessage duplicates info already in the error chain — The method/URL path extracted in gitHubErrorMessage() is the same info that github.ErrorResponse.Error() already formats. This means in wrapGitHubError lines 392/395, the wrapped error message will contain this info twice (once in the status message, once in the wrapped error). Not a bug, but worth being aware of for log readability.

---

[johnallers]

Closing in favor of #132