fix(http): avoid marking nodes down on client errors by lemonel · Pull Request #360 · apolloconfig/agollo

lemonel · 2026-05-28T08:02:48Z

Summary

将 4xx 响应包装为 clientRequestInvalidError
RequestRecovery() 遇到这类错误时不再调用 SetDownNode
保留 5xx、超时、网络错误的原有降级和下线逻辑

Why

之前 404 等 4xx 会被当成节点故障处理，导致单个 namespace 的请求失败污染整台节点状态
这会影响后续对其他 namespace 的拉取，出现空配置且无任何提示（如启动apollo后后续通过GetConfig(namespace string) *storage.Config，如果先获取不存在的namespace，获取其他已经存在的namespace仍然会获取到nil config，且无法感知）

Summary by CodeRabbit

Bug Fixes
- Improved handling of client-side HTTP errors (400, 401, 403, 404, 405) by more accurately classifying invalid requests and preventing targets from being incorrectly marked unavailable.
Tests
- Expanded and table-driven coverage for HTTP fail-fast behavior across multiple status codes, including expected error classification, timing, and whether the target is marked down.

coderabbitai · 2026-05-28T08:03:04Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f960420d-e65e-45df-bfed-f9a92accac0c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

✅ Review completed - (🔄 Check again to review again)

📝 Walkthrough

Walkthrough

This PR introduces a typed error for client-side HTTP failures (400/401/404/405) to prevent unnecessary server downtime marking. The Request function now returns clientRequestInvalidError for these status codes, and RequestRecovery detects this error and returns immediately instead of marking the server as down. Tests are expanded to validate error classification and expected recovery behavior across multiple status codes.

Changes

Client Request Error Differentiation

Layer / File(s)	Summary
Typed client request error and fail-fast detection `protocol/http/request.go`	Adds `clientRequestInvalidError` type with status code and `isClientRequestInvalidError` helper. `Request` returns this typed error for 400/401/404/405 responses. `RequestRecovery` detects the typed error and returns immediately without marking the server down.
Test validation of error classification and recovery behavior `protocol/http/request_test.go`	`TestFailFastStatusCode` refactored into parameterized table-driven test with expectations for error classification, duration reporting, and server downtime. Helper assertions validate `isClientRequestInvalidError` match, duration values, and `IsDown` state.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A rabbit hops through error states so clear,
Client faults now fail fast without a tear,
No servers marked down for bad requests in sight,
Just typed errors pointing the true path right! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(http): avoid marking nodes down on client errors' directly and accurately describes the main change in the PR - preventing 4xx client errors from marking nodes as down.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

mergify · 2026-05-28T08:04:43Z

感谢您提出Pull Request，我会尽快Review。我会在1-2日内进行查看或者回复，如果遇到节假日可能会处理较慢，敬请谅解。

nobodyiam · 2026-05-30T02:40:53Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf289294da

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-30T02:43:44Z

 		case http.StatusBadRequest, http.StatusUnauthorized, http.StatusNotFound, http.StatusMethodNotAllowed:
 			log.Errorf("Connect Apollo Server Fail, url:%s, StatusCode:%d", requestURL, res.StatusCode)
-			return nil, errors.New(fmt.Sprintf("Connect Apollo Server Fail, StatusCode:%d", res.StatusCode))
+			return nil, &clientRequestInvalidError{statusCode: res.StatusCode}


Treat forbidden responses as client errors

When Apollo returns another client-side status such as 403 Forbidden for an unauthorized app/namespace, this still falls through to the retrying default case and RequestRecovery will call SetDownNode, marking a healthy node down for a bad request/permission issue. Apollo documents 403 as a client authorization error alongside 400/401/404/405 (https://github.com/apolloconfig/apollo/wiki/Apollo%E5%BC%80%E6%94%BE%E5%B9%B3%E5%8F%B0#%E5%9B%9B%E9%94%99%E8%AF%AF%E7%A0%81%E8%AF%B4%E6%98%8E), so this fix should classify the full non-retryable 4xx range (or at least include 403) before marking nodes down.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR adjusts Apollo HTTP request handling to treat specific 4xx responses as client-side request errors, preventing those responses from incorrectly triggering node down-marking and cascading impact to other namespaces.

Changes:

Introduces a dedicated clientRequestInvalidError for selected 4xx status codes (400/401/404/405).
Updates RequestRecovery() to avoid calling server.SetDownNode(...) when the failure is a client-side 4xx error.
Expands status-code tests to validate both “marked down” behavior and the new invalid-request classification.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
protocol/http/request.go	Adds a typed error for selected 4xx responses and prevents node down-marking for those errors in recovery logic.
protocol/http/request_test.go	Extends fail-fast status code tests to cover both client (4xx) and server (5xx) behaviors, including node down state assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+func (e *clientRequestInvalidError) Error() string {
+	return fmt.Sprintf("Connect Apollo Server Fail, StatusCode:%d", e.statusCode)
+}


+	if expectedDurationZero {
+		Assert(t, int64(0), Equal(duration))
+	} else {
+		Assert(t, int64(10), Equal(duration))
+	}


mergify · 2026-07-19T12:47:42Z

这个 PR 发生了冲突. @lemonel 是否能帮忙修复呢? 🙏

zouyx · 2026-07-19T13:01:40Z

@copilot resolve the merge conflicts in this pull request

fix(http): avoid marking nodes down on client errors

bf28929

chatgpt-codex-connector Bot reviewed May 30, 2026

View reviewed changes

zouyx changed the base branch from master to develop June 15, 2026 01:27

zouyx requested a review from Copilot July 18, 2026 16:18

Copilot started reviewing on behalf of zouyx July 18, 2026 16:18 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

fix(http): handle forbidden responses as client errors

3513c16

Merge branch 'develop' into dev/fix_unexpected_down_mark

c053711

zouyx approved these changes Jul 19, 2026

View reviewed changes

zouyx merged commit dd76cce into apolloconfig:develop Jul 19, 2026
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(http): avoid marking nodes down on client errors - #360

fix(http): avoid marking nodes down on client errors#360
zouyx merged 3 commits into
apolloconfig:developfrom
lemonel:dev/fix_unexpected_down_mark

lemonel commented May 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 28, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

mergify Bot commented May 28, 2026

Uh oh!

nobodyiam commented May 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mergify Bot commented Jul 19, 2026

Uh oh!

zouyx commented Jul 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

lemonel commented May 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

mergify Bot commented May 28, 2026

Uh oh!

nobodyiam commented May 30, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mergify Bot commented Jul 19, 2026

Uh oh!

zouyx commented Jul 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lemonel commented May 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 28, 2026 •

edited

Loading