Skip to content

Use the response factory instead of Guzzle for error response - #80

Merged
clue merged 1 commit into
ratchetphp:masterfrom
mbabker:fix-incorrect-dev-dependency
Jul 28, 2026
Merged

Use the response factory instead of Guzzle for error response#80
clue merged 1 commit into
ratchetphp:masterfrom
mbabker:fix-incorrect-dev-dependency

Conversation

@mbabker

@mbabker mbabker commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

This package declares guzzlehttp/psr7 as a dev-only dependency, yet, the Ratchet\RFC6455\Handshake\ServerNegotiator class is directly using it in an error handling branch to create a new response. This PR replaces the incorrect use of the dev-only dependency with the response factory in this code branch.

@clue clue left a comment

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.

@mbabker Thanks for tracking this down, fix looks right to me.

This finishes off #34 which converted the other branches to the PSR-17 factory but missed this one after catching up with #23.

Could you add a test for it before we merge? That branch has no coverage today, and because the suite injects Guzzle's own HttpFactory a plain status assertion passes either way, so it has to assert the factory is actually used. This goes red on master and green with your change:

$responseFactory = $this->createMock(ResponseFactoryInterface::class);
$responseFactory->expects($this->exactly(2))
    ->method('createResponse')
    ->willReturnOnConsecutiveCalls(
        (new HttpFactory())->createResponse(),
        (new HttpFactory())->createResponse(400, 'server_max_window_bits must have a value between 8 and 15.')
    );

paired with a request carrying Sec-WebSocket-Extensions: permessage-deflate; server_max_window_bits=99.

@clue clue added this to the 0.4.2 milestone Jul 27, 2026
@clue clue added the bug label Jul 27, 2026
@mbabker
mbabker force-pushed the fix-incorrect-dev-dependency branch from 84a1df8 to 3981ec5 Compare July 27, 2026 12:30
@mbabker

mbabker commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Tests updated

@clue clue left a comment

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.

@mbabker Thanks for the update, changes LGTM :shipit:

@clue
clue merged commit dc19074 into ratchetphp:master Jul 28, 2026
14 checks passed
@mbabker
mbabker deleted the fix-incorrect-dev-dependency branch July 28, 2026 12:33
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.

2 participants