Skip to content

Conversation

@lucasnetau
Copy link
Contributor

The expected error code when ext-sockets is not enabled (SOCKET_EMSGSIZE is not defined) is 90 per the two places that OverFlowException is created in class Transaction

returnreject(new \OverflowException(
'Response body size of ' . $size . ' bytes exceeds maximum of ' . $this->maximumSize . ' bytes',
\defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90
));

$reject(new \OverflowException(
'Response body size exceeds maximum of ' . $this->maximumSize . ' bytes',
\defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90
));

@SimonFrings
Copy link
Member

@lucasnetau Thanks for bringing this up 👍

Seems like a reasonable change to me, but I don't have at much insights on this as @clue and @WyriHaximus, so interested in their opinion on this.

Copy link
Member

@clueclue left a comment

Choose a reason for hiding this comment

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

@lucasnetau Thank you for looking into this, changes LGTM! :shipit:

This fixes a small oversight in our test suite only that I've introduced when removing the dependency on reactphp/promise-stream (#482).

Once merged, we should probably backport the same changes to the 1.x branch as well 👍

@clueclue added this to the v3.0.0 milestone Aug 10, 2024
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Aug 27, 2024
This is a backport of reactphp#532 and corrects an oversight introduced in reactphp#482.
@WyriHaximus
Copy link
Member

Once merged, we should probably backport the same changes to the 1.x branch as well 👍

Just filed #539 to address this in the 1.x branch.

Copy link
Member

@WyriHaximusWyriHaximus left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to file this fix, LGTM 👍

@WyriHaximusWyriHaximus merged commit fc09a64 into reactphp:3.xAug 27, 2024
WyriHaximus pushed a commit to WyriHaximus-labs/http that referenced this pull request Aug 30, 2024
This is a backport of reactphp#532 and corrects an oversight introduced in reactphp#482.
Sign up for freeto 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.

4 participants

@lucasnetau@SimonFrings@WyriHaximus@clue