Skip to content

fix: log errors at the http level - #15

Merged
thedadams merged 1 commit into
obot-platform:mainfrom
thedadams:push-wkruqsylqyxm
Sep 14, 2026
Merged

thedadams merged 1 commit into
obot-platform:mainfrom
thedadams:push-wkruqsylqyxm

Conversation

@thedadams

Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Donnie Adams <donnie@obot.ai>
Copilot AI balanced review requested due to automatic review settings September 14, 2026 14:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is limited in scope; the requested regression test is a minor nit.

Pull request overview

Adds HTTP-level logging for frontend identity resolution failures.

Changes:

  • Logs registry errors through the configured logger.
  • Preserves authorization capture and HTTP 500 behavior.
File summaries
File Summary
http.go Logs frontend identity resolution failures.
Review details

Suppressed comments (1)

http.go:66

  • This adds the PR's observable behavior, but no HTTP test exercises the registry-failure path or verifies that the configured logger receives the error. Please add a regression test with a capturing slog.Handler and a discovery failure so this logging contract cannot regress silently.
			if logger := c.serverOptions.Logger; logger != nil {
				logger.ErrorContext(r.Context(), "resolving frontend identity", "error", err)
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@thedadams
thedadams merged commit 1c2a345 into obot-platform:main Sep 14, 2026
4 checks passed
@thedadams
thedadams deleted the push-wkruqsylqyxm branch September 14, 2026 14:59
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.

3 participants