Skip to content

Enhance error handling in PendingRequest to convert TooManyRedirectsE… #55998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 11, 2025

Conversation

achrafAa
Copy link
Contributor

Fix #55989 TooManyRedirectsException handling in HTTP client

Problem

Since Laravel v12.18.0, when an HTTP request exceeds the maximum number of redirects, a TooManyRedirectsException from Guzzle causes a PHP Error to be thrown with the message "Can only throw objects" instead of the expected ConnectionException.

This breaks existing applications that rely on catching ConnectionException for redirect limit handling, particularly for health checks and similar use cases.

Before this fix:

Http::fake([
    '1.example.com' => Http::response(null, 301, ['Location' => 'https://2.example.com']),
    '2.example.com' => Http::response(null, 301, ['Location' => 'https://3.example.com']),
    '3.example.com' => Http::response('', 200),
]);

// This throws: Error: "Can only throw objects"
Http::createPendingRequest()
    ->maxRedirects(1)
    ->get('https://1.example.com');

After this fix:

// This correctly throws: Illuminate\Http\Client\ConnectionException
Http::createPendingRequest()
    ->maxRedirects(1)
    ->get('https://1.example.com');

Root Cause

The issue occurs in PendingRequest::marshalRequestExceptionWithResponse() method:

  1. When TooManyRedirectsException is thrown by Guzzle, it includes the last redirect response (typically 301/302 status)
  2. The method calls $response->toException() which only returns a RequestException for 4xx/5xx errors
  3. For 3xx redirect responses, toException() returns null because failed() returns false
  4. The method then tries to throw null, causing the PHP Error

Solution

Modified marshalRequestExceptionWithResponse() to handle the case where toException() returns null:

protected function marshalRequestExceptionWithResponse(RequestException $e)
{
    $this->factory?->recordRequestResponsePair(
        new Request($e->getRequest()),
        $response = $this->populateResponse($this->newResponse($e->getResponse()))
    );

    $exception = $response->toException();

    if ($exception === null) {
        // For cases like TooManyRedirectsException where the response isn't an error status code
        $exception = new ConnectionException($e->getMessage(), 0, $e);
    }

    throw $exception;
}

This ensures that:

  • TooManyRedirectsException and similar exceptions are properly wrapped as ConnectionException
  • The original Guzzle exception and message are preserved
  • Normal 4xx/5xx error handling remains unchanged

Tests Added

  • testTooManyRedirectsExceptionConvertedToConnectionException: Tests direct Guzzle exception handling
  • testTooManyRedirectsWithFakedRedirectChain: Tests the scenario from the original issue using faked responses

Impact

  • Bug Fix: Resolves "Can only throw objects" error
  • Backward Compatible: Existing code will work as expected
  • Type Safety: Proper exception hierarchy maintained

Related to: Laravel v12.18.0 Guzzle exception handling refactoring

achrafAa added 2 commits June 11, 2025 21:40
…xception to ConnectionException. Add tests to verify the conversion of redirect exceptions.
…ing logic by using null coalescing operator.
@taylorotwell taylorotwell merged commit 85f7487 into laravel:12.x Jun 11, 2025
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TooManyRedirectsException in Http facade causes Error instead of throwing ConnectionException
2 participants