refactor(grpc): improve Parser::parseResponse return value format by huangdijia · Pull Request #7653 · hyperf/hyperf

huangdijia · 2025-12-01T14:10:28Z

Summary

Refactored the Parser::parseResponse method to use a more standardized approach
Replaced Grpc\StringifyAble dependency with stdClass for better compatibility
Updated return type annotation to be more explicit about the return structure
Standardized all return paths to use a consistent stdClass status object

Changes Made

Import Change: Replaced use Grpc\StringifyAble; with use stdClass;
Return Type: Updated PHPDoc to clearly define the return structure as array{0:null|Message,1:stdClass{code:int,details:string,metadata:null|Http2Response}}
Code Structure: All return scenarios now use a consistent stdClass status object with:
- code: Error/status code
- details: Descriptive message
- metadata: Response metadata

Test Plan

Verify existing gRPC functionality continues to work
Test error handling scenarios (no response, invalid status, grpc errors)
Test successful response parsing
Ensure compatibility with existing code that uses this method

Copilot

Pull request overview

This PR refactors the Parser::parseResponse method to return a more standardized format using stdClass instead of a mixed-type array. The change replaces the old three-element array format [message, code, response] with a two-element array [message, statusObject] where the status object contains code, details, and metadata fields.

Key Changes:

Replaced Grpc\StringifyAble import with stdClass
Updated return type annotation to specify structured stdClass status object
Standardized all return paths to use consistent stdClass format with code, details, and metadata fields

Reviewed changes

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

File	Description
src/grpc/src/Parser.php	Refactored `parseResponse` method to return structured stdClass status object instead of mixed array elements
CHANGELOG-3.2.md	Added changelog entry documenting the improvement

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

Copilot

Pull request overview

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

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

Copilot · 2025-12-02T00:41:33Z

+        return [
+            self::deserializeMessage($deserialize, $response->data ?? ''),
+            self::statusFromResponse($response),
+            $response,
+        ];


Critical error handling logic has been removed without replacement. The old implementation checked for:

Invalid HTTP status codes (via isInvalidStatus()) - returning error when statusCode was not 0, 200, or 400

Non-zero gRPC status codes (grpc-status !== 0) - returning the gRPC error message

The new implementation only calls statusFromResponse() which only extracts grpc-status-details-bin header. This means:

Invalid HTTP status codes are no longer detected or reported

gRPC error status codes (from grpc-status header) are not being checked or returned

Error messages from grpc-message header are lost

The method now deserializes the response data unconditionally, even when there are errors, which could lead to deserialization failures or incorrect behavior. The error handling logic should be restored to properly validate the response before attempting deserialization.

- Replace Grpc\StringifyAble with stdClass for better standardization - Update return type annotation to be more explicit about the structure - Standardize all return paths to use consistent stdClass status object - Improve code readability and maintainability

… standardization

Co-authored-by: Copilot <[email protected]>

…Status object

…onse

Co-authored-by: Copilot <[email protected]>

…s codes and messages

…eResponse

…Call class

…alize parameters in UnaryCall class

…it method

…in wait method

…streamId directly

…cases

…tatus details

…organization

…r organization

…ethod in UnaryCall class

Copilot

Pull request overview

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

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

Copilot · 2025-12-02T11:03:46Z

+            if (in_array($lowerKey, ['content-type', 'content-length', 'te'])) {
+                continue;
+            }
+            // 处理-bin结尾 metadata


[nitpick] The comment contains Chinese characters which may not be clear to all contributors. Consider translating to English: "Handle -bin suffixed metadata"

Suggested change

// 处理-bin结尾 metadata

// Handle -bin suffixed metadata

Copilot · 2025-12-02T11:03:47Z

+        $status = new stdClass();
+        $status->code = 0;
+        $status->details = 'OK';
+        $status->metadata = [];


The metadata is initialized as an empty array on line 57, but should be initialized to match the documented return type. According to the PHPDoc annotations, metadata should be typed as null|Http2Response in the return type (lines 23, 35, 50), not as an array. Either change line 57 to $status->metadata = null; or update the PHPDoc to reflect that metadata is an array of parsed headers.

Suggested change

$status->metadata = [];

$status->metadata = null;

Copilot · 2025-12-02T11:03:47Z

+            if (str_starts_with($lowerKey, 'grpc-') && $lowerKey !== 'grpc-status-details-bin') {
+                continue;
+            }
+            // 忽略http2预留伪头


[nitpick] The comment contains Chinese characters which may not be clear to all contributors. Consider translating to English: "Ignore HTTP/2 reserved pseudo-headers"

Suggested change

// 忽略http2预留伪头

// Ignore HTTP/2 reserved pseudo-headers

Copilot · 2025-12-02T11:03:47Z

+            if (str_starts_with($lowerKey, ':')) {
+                continue;
+            }
+            // 忽略 HTTP/2 传输层头部


[nitpick] The comment contains Chinese characters which may not be clear to all contributors. Consider translating to English: "Ignore HTTP/2 transport layer headers"

Suggested change

// 忽略 HTTP/2 传输层头部

// Ignore HTTP/2 transport layer headers

Copilot · 2025-12-02T11:03:48Z

        }, $this->options['retry_interval'] ?? 100);
-        return Parser::parseResponse($this->recv($streamId), $deserialize);
+
+        return new UnaryCall($this, $streamId, $deserialize);


This change breaks existing tests. The method now returns a UnaryCall object instead of directly parsing and returning the response. However, BaseClientTest.php expects _simpleRequest to return a numeric value (stream ID) in tests like testGrpcClientReconnect() which calls $this->assertGreaterThan(0, $client->sayHello()). Since sayHello() returns the result of _simpleRequest, these tests will fail. The tests need to be updated to call ->wait() on the returned UnaryCall object, or test coverage should be added for the new return behavior.

Copilot · 2025-12-02T11:03:48Z

+        $status->code = 0;
+        $status->details = 'OK';
+        $status->metadata = [];
+        $status->rawResponse = $response;


The rawResponse property is being added to the status object but is not documented in the return type annotation. The PHPDoc at line 35 and 50 should include rawResponse in the stdClass structure definition: stdClass{code:int,details:string,metadata:array,rawResponse:null|Http2Response}

Copilot · 2025-12-02T11:03:48Z

+
+        foreach ($response->headers as $key => $value) {
+            $lowerKey = strtolower($key);
+            // 忽略grpc官方预留，将grpc-status-details-bin保留，可解析为Google\Rpc\Status


[nitpick] The comment contains Chinese characters which may not be clear to all contributors. Consider translating to English: "Ignore official gRPC reserved headers, keep grpc-status-details-bin which can be parsed as Google\Rpc\Status"

Suggested change

// 忽略grpc官方预留，将grpc-status-details-bin保留，可解析为Google\Rpc\Status

// Ignore official gRPC reserved headers, keep grpc-status-details-bin which can be parsed as Google\Rpc\Status

Co-authored-by: Copilot <[email protected]>

huangdijia requested a review from limingxinleo December 1, 2025 14:12

huangdijia marked this pull request as draft December 1, 2025 14:41

huangdijia requested review from Copilot and removed request for limingxinleo December 1, 2025 14:50

Copilot started reviewing on behalf of huangdijia December 1, 2025 14:51 View session

huangdijia added this to the v3.2 milestone Dec 1, 2025

Copilot finished reviewing on behalf of huangdijia December 1, 2025 14:52

Copilot AI reviewed Dec 1, 2025

View reviewed changes

Comment thread src/grpc/src/Parser.php Outdated

Comment thread src/grpc/src/Parser.php Outdated

Comment thread src/grpc/src/Parser.php Outdated

huangdijia requested a review from Copilot December 2, 2025 00:38

Copilot started reviewing on behalf of huangdijia December 2, 2025 00:39 View session

Copilot finished reviewing on behalf of huangdijia December 2, 2025 00:40

Copilot AI reviewed Dec 2, 2025

View reviewed changes

limingxinleo approved these changes Dec 2, 2025

View reviewed changes

huangdijia and others added 17 commits December 2, 2025 15:45

refactor(grpc): update parseResponse return type metadata to array

bed08ad

refactor(grpc): improve Parser::parseResponse return value format for…

67681c8

… standardization

更新 Parser.php

49a6330

Co-authored-by: Copilot <[email protected]>

refactor(grpc): standardize parseResponse return value format to use …

adda503

…Status object

refactor(grpc): update parseResponse return type to include Http2Resp…

c43bc09

…onse

更新 CHANGELOG-3.2.md

b541291

Co-authored-by: Copilot <[email protected]>

更新 Parser.php

01f5b7e

Co-authored-by: Copilot <[email protected]>

refactor(grpc): enhance parseResponse to handle additional gRPC statu…

2b1ad93

…s codes and messages

refactor(grpc): update error message for gRPC status handling in pars…

a9daf85

…eResponse

refactor(grpc): replace response parsing in _simpleRequest with Unary…

f4c0bf2

…Call class

refactor(grpc): fix status message assignment in UnaryCall class

975a0d9

refactor(grpc): update constructor visibility for response and deseri…

29fb73f

…alize parameters in UnaryCall class

refactor(grpc): defer response parsing in UnaryCall constructor to wa…

d5e988f

…it method

refactor(grpc): simplify UnaryCall constructor and response handling …

7f01c11

…in wait method

refactor(grpc): update UnaryCall instantiation in BaseClient to pass …

a64996a

…streamId directly

refactor(grpc): add wait method to info and getFeature calls in test …

899a155

…cases

huangdijia added 8 commits December 2, 2025 15:45

refactor(grpc): enhance metadata parsing in UnaryCall class

2a23528

refactor(grpc): initialize parsed property as null in UnaryCall class

339cc4a

refactor(grpc): enhance response parsing in Parser class to include s…

a809d12

…tatus details

Revert

bc997d8

refactor(grpc): move parseMetadata method to Parser class for better …

0a58868

…organization

refactor(grpc): move isInvalidStatus method to Parser class for bette…

bd0ab6f

…r organization

refactor(grpc): update parseResponse method to handle metadata as arrays

fb614bd

refactor(grpc): add PHPDoc annotations for parsed property and wait m…

11619d9

…ethod in UnaryCall class

huangdijia requested review from Copilot and limingxinleo December 2, 2025 10:57

huangdijia marked this pull request as ready for review December 2, 2025 10:57

Copilot started reviewing on behalf of huangdijia December 2, 2025 10:58 View session

Copilot finished reviewing on behalf of huangdijia December 2, 2025 11:01

Copilot AI reviewed Dec 2, 2025

View reviewed changes

limingxinleo approved these changes Dec 2, 2025

View reviewed changes

更新 Parser.php

d747c10

Co-authored-by: Copilot <[email protected]>

limingxinleo merged commit 5b71af8 into hyperf:3.2 Dec 2, 2025

huangdijia deleted the refactor/grpc-parser-response-format branch December 2, 2025 11:49

	// 处理-bin结尾 metadata
	// Handle -bin suffixed metadata

	// 忽略http2预留伪头
	// Ignore HTTP/2 reserved pseudo-headers

	// 忽略 HTTP/2 传输层头部
	// Ignore HTTP/2 transport layer headers

	// 忽略grpc官方预留，将grpc-status-details-bin保留，可解析为Google\Rpc\Status
	// Ignore official gRPC reserved headers, keep grpc-status-details-bin which can be parsed as Google\Rpc\Status

Conversation

huangdijia commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes Made

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

huangdijia commented Dec 1, 2025 •

edited

Loading