Skip to content

Fix nil error logging in probe registration#874

Merged
afritzler merged 3 commits into
mainfrom
worktree-issue_fixes
May 26, 2026
Merged

Fix nil error logging in probe registration#874
afritzler merged 3 commits into
mainfrom
worktree-issue_fixes

Conversation

@stefanhipfel
Copy link
Copy Markdown
Contributor

@stefanhipfel stefanhipfel commented May 11, 2026

Fixes #785

Summary by CodeRabbit

  • Bug Fixes
    • Improved server registration error reporting: failures now include HTTP status codes and response body (or read error) to aid troubleshooting.
    • Replaced generic registration failure logs with detailed, formatted error messages for clearer diagnostics.

Review Change Stack

@stefanhipfel stefanhipfel requested a review from a team as a code owner May 11, 2026 08:22
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Warning

Rate limit exceeded

@stefanhipfel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 46 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6dd6df48-7096-43ab-9cc8-d49d57320ed5

📥 Commits

Reviewing files that changed from the base of the PR and between f9f6ef8 and 8fd18f7.

📒 Files selected for processing (1)
  • internal/probe/probe.go
📝 Walkthrough

Walkthrough

The probe agent's registerServer now captures HTTP response status and up to 1024 bytes of the response body on non-200/201 replies, and logs a formatted error containing that information while keeping retry/backoff behavior unchanged.

Changes

Server Registration Error Logging

Layer / File(s) Summary
Import dependencies
internal/probe/probe.go
Adds fmt and io imports to enable formatted error messages and limited response-body reading.
Error handling with response body
internal/probe/probe.go
On non-200/201 HTTP responses, reads up to 1024 bytes from the response body and logs an error including the HTTP status code and either the read failure or the response text; returns (false, nil) to preserve retry logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • afritzler
  • nagadeesh-nagaraja
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description only contains 'Fixes #785' and lacks details about the proposed changes. The template requires sections for 'Proposed Changes' with bullet points. Expand the description to include specific proposed changes and implementation details that explain how the nil error logging issue is resolved.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix nil error logging in probe registration' accurately describes the main change—improving error logging in the probe registration function by handling nil errors.
Linked Issues check ✅ Passed The PR changes address the core requirement from issue #785: reading response body and logging a concrete error with status code when HTTP registration returns non-OK responses.
Out of Scope Changes check ✅ Passed All changes in internal/probe/probe.go directly address the nil error logging issue in registerServer from issue #785, with no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-issue_fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added size/XS bug Something isn't working labels May 11, 2026
Fixes #785

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
@stefanhipfel stefanhipfel force-pushed the worktree-issue_fixes branch from ee5642e to 1ecf4b0 Compare May 11, 2026 08:23
Comment thread internal/probe/probe.go
@@ -221,7 +223,8 @@ func (a *Agent) registerServer(ctx context.Context) error {
}()

if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@afritzler do we want to retry on all errors? 401 or 403 for example won't fix themselves

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO I would keep it simple for now. But you are are of course right.

@afritzler
Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/probe/probe.go`:
- Around line 226-227: The call to io.ReadAll(io.LimitReader(resp.Body, 1024))
currently ignores its returned error which can hide read failures; modify the
code around the resp handling (where body, _ :=
io.ReadAll(io.LimitReader(resp.Body, 1024)) and the subsequent a.log.Error call
occur) to capture the read error (e.g., body, err := io.ReadAll(...)), and
include that error in the a.log.Error invocation or its message (alongside
resp.StatusCode, the body string and "url", a.RegistryURL) so read failures are
reported instead of discarded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b98ae21-b03a-4bd6-a547-be75c21e6bed

📥 Commits

Reviewing files that changed from the base of the PR and between bd30487 and 1ecf4b0.

📒 Files selected for processing (1)
  • internal/probe/probe.go

Comment thread internal/probe/probe.go Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
@stefanhipfel stefanhipfel force-pushed the worktree-issue_fixes branch from f9f6ef8 to ef8ba72 Compare May 15, 2026 07:59
@github-actions github-actions Bot added size/XS and removed size/S labels May 19, 2026
Remove duplicate return statement on line 233 that was causing
govet unreachable code error. The duplicate return was accidentally
added and made the code after it unreachable.

Signed-off-by: Stefan Hipfel <stefan.hipfel@sap.com>
@stefanhipfel stefanhipfel force-pushed the worktree-issue_fixes branch from 81cefd5 to 8fd18f7 Compare May 19, 2026 12:18
@afritzler afritzler merged commit 943033e into main May 26, 2026
17 checks passed
@afritzler afritzler deleted the worktree-issue_fixes branch May 26, 2026 07:34
@github-project-automation github-project-automation Bot moved this to Done in Roadmap May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug: nil error logged on non-OK HTTP response in probe agent registerServer

4 participants