Skip to content

Conversation

@CAICAIIs
Copy link
Contributor

@CAICAIIs CAICAIIs commented Sep 7, 2025

Description

This PR fixes RPC method signature validation warnings for health server methods and improves server configuration handling.

Problem

The following warnings were being generated due to incorrect RPC method signatures:

2025-09-04 15:25:16 WARN common/rpc_service.go:382 method Resume of mtype func(health.HealthTripleServer) has wrong number of in out parameters 0; needs exactly 1/2
2025-09-04 15:25:16 WARN common/rpc_service.go:382 method SetServingStatus of mtype func(health.HealthTripleServer, string, triple_health.HealthCheckResponse_ServingStatus) has wrong number of in out parameters 0; needs exactly 1/2
2025-09-04 15:25:16 WARN common/rpc_service.go:382 method Shutdown of mtype func(health.HealthTripleServer) has wrong number of in out parameters 0; needs exactly 1/2

These warnings occur because the RPC service validation in common/rpc_service.go requires methods to have 1 or 2 return values (with the last one being error), but the health server methods had no return values.

Additionally, unnecessary warnings about missing service configurations were being shown when using the new server API without config files.

Related to apache/dubbo-go-samples#910

…ings

- Add error return type to HealthTripleServer methods (Resume, SetServingStatus, Shutdown)
- Fix RPC method signature validation warnings
- Only warn about missing service config when services are actually configured
- Improve compatibility with new server API without config files
@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.63%. Comparing base (60d1c2a) to head (9accb9e).
⚠️ Report is 632 commits behind head on develop.

Files with missing lines Patch % Lines
server/server.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3018      +/-   ##
===========================================
- Coverage    46.76%   39.63%   -7.14%     
===========================================
  Files          295      457     +162     
  Lines        17172    38884   +21712     
===========================================
+ Hits          8031    15410    +7379     
- Misses        8287    22216   +13929     
- Partials       854     1258     +404     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes RPC method signature validation warnings for health server methods and reduces unnecessary configuration warnings when using the new server API without config files.

  • Adds error return values to health server methods (Resume, SetServingStatus, Shutdown) to comply with RPC validation requirements
  • Improves configuration handling to avoid warning about missing service configs when no services are configured
  • Updates RPC service validation to exclude health helper methods from signature checks

Reviewed Changes

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

File Description
protocol/triple/health/healthServer.go Adds error return values to Resume, SetServingStatus, and Shutdown methods
server/server.go Conditionally shows service config warnings only when services are actually configured
common/rpc_service.go Excludes health helper methods from RPC validation checks
Comments suppressed due to low confidence (2)

protocol/triple/health/healthServer.go:1

  • The SetServingStatus method now returns an error, but this call ignores the return value. The error should be handled or returned to the caller.
/*

protocol/triple/health/healthServer.go:1

  • The SetServingStatus method now returns an error, but this call ignores the return value. The error should be handled or returned to the caller.
/*

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@CAICAIIs CAICAIIs force-pushed the fix/health-server-method-signatures branch from 00b0e91 to 7821627 Compare September 9, 2025 06:12
@CAICAIIs CAICAIIs changed the title fix: add error return to health server methods and reduce config warnings fix: reduce config warnings Sep 9, 2025
@sonarqubecloud
Copy link

Copy link
Member

@marsevilspirit marsevilspirit left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@1kasa 1kasa left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexStocks AlexStocks merged commit c591c87 into apache:develop Sep 20, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants