Skip to content

fix(events): annotate ListEvents errors with status/request id/window#165

Open
arreyder wants to merge 2 commits into
mainfrom
chrisrhodes/ops-1539-baton-okta-listevents-error-wrapping-drops-http-status
Open

fix(events): annotate ListEvents errors with status/request id/window#165
arreyder wants to merge 2 commits into
mainfrom
chrisrhodes/ops-1539-baton-okta-listevents-error-wrapping-drops-http-status

Conversation

@arreyder
Copy link
Copy Markdown

Fixes OPS-1539.

Summary

  • The vendored okta-sdk-golang *okta.Error formatter returns the literal string "the API returned an unknown error" whenever neither ErrorDescription nor ErrorSummary is populated (HTML 5xx pages, gateway timeouts, non-JSON bodies). It also drops HTTP status and X-Okta-Request-Id. Errors reaching prod traces were undiagnosable.
  • Wrap LogEvent.GetLogs errors with HTTP status, X-Okta-Request-Id, and the request's since / after query window. Uses %w so callers can still errors.Is / errors.As against the inner *okta.Error.
  • Lifts the wrapping logic into a private wrapListEventsError helper so we have a single place to extend for additional Okta call sites later (the OPS-1539 ticket flags this as a follow-up across other *Okta SDK calls).

Test plan

  • go test -count=1 -short ./pkg/connector/... passes locally
  • New event_log_test.go covers:
    • Full-context wrap (status + request_id + window) round-trips through the error message
    • Nil response (early network failure) still produces a usable message anchored to the query window
    • Missing optional fields (no request-id header, no cursor) are elided cleanly with no dangling key=
    • errors.Is(wrapped, inner) continues to hold
  • After merge, re-query the okta error spans and confirm "the API returned an unknown error" is now prefixed with usable context

Follow-ups (not in this PR)

  • Apply the same wrap to other *Okta SDK call sites for consistency once we have a second consumer of wrapListEventsError to justify generalizing it.
  • Consider patching the formatter in the ConductorOne/okta-sdk-golang fork to include status + body by default — the "unknown error" sentinel is a known footgun.

🤖 Generated with Claude Code

…ndow

The vendored okta-sdk-golang *okta.Error formatter returns the literal
string "the API returned an unknown error" when neither ErrorDescription
nor ErrorSummary is populated (HTML 5xx pages, gateway timeouts, non-JSON
bodies). It also drops HTTP status and X-Okta-Request-Id. Errors arriving
in prod traces were undiagnosable.

Wrap GetLogs errors with HTTP status, X-Okta-Request-Id, and the request's
`since` / `after` query window. Uses %w so callers can still errors.Is /
errors.As the inner *okta.Error.

Adds event_log_test.go covering:
- Full-context wrap (status + request_id + window)
- Nil response (early network failure)
- Missing optional fields elided cleanly

Refs OPS-1539

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arreyder arreyder requested a review from a team May 14, 2026 14:38
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 14, 2026

OPS-1539

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 14, 2026

Connector PR Review: fix(events): annotate ListEvents errors with status/request id/window

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Review mode: incremental since 06d2893

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

…bvars)

CI lint (usestdlibvars) flagged the literal `502` and `500` HTTP status
codes in the wrap-error test fixtures; replace with `http.StatusBadGateway`
and `http.StatusInternalServerError`. Behavior unchanged.

Refs OPS-1539

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant