Skip to content

Show the real error when a 4xx carries no validation bag - #285

Closed
datashaman wants to merge 2 commits into
laravel:masterfrom
datashaman:fix-validation-error-display
Closed

datashaman wants to merge 2 commits into
laravel:masterfrom
datashaman:fix-validation-error-display

Conversation

@datashaman

Copy link
Copy Markdown
Contributor

displayValidationErrors() assumes every 400 / 422 carries a Laravel validation bag:

$errors = collect(json_decode((string) $response->getBody(), true)['errors'])->flatten();

When the body is empty or is not JSON, json_decode returns null, the offset access emits Trying to access array offset on null, and the user is left with a bare Whoops! There were some problems with your request. followed by nothing. The response that would have explained the failure is discarded.

PHP Warning:  Trying to access array offset on null in .../src/ConsoleVaporClient.php on line 1541

Whoops! There were some problems with your request.

That is the entire output. There is nothing to act on and nothing to search for.

We hit this on production deploys, where the first call vapor deploy makes — POST /api/projects/{id}/environments/{env}/linted-manifest — started coming back as an empty-bodied 4xx, so every deploy died about a second in with only the message above. Diagnosing it meant reading the CLI source and replaying the request by hand.

What this changes

displayValidationErrors() now falls back through the response instead of assuming its shape:

  1. the validation bag, as today
  2. a top-level message
  3. the raw body, truncated to 500 characters, for HTML or plain-text responses
  4. the status code, when the body is empty

The validation-bag path is unchanged, so normal output is identical.

Incidental observation, not fixed here

The empty-bodied 4xx began for us the moment v1.70.4 was installed, which widened the guzzle constraint to allow ^8.0 (#284) and moved us from guzzle 7.15.5 to 8.1.0. Replaying the request under both versions against a socket, the bytes are identical apart from User-Agent: GuzzleHttp/7 becoming GuzzleHttp/8. Pinning back to 1.70.3 restored deploys.

I can't see the API side, so I can't say whether the User-Agent is what is being rejected — flagging it in case it's useful. #284's test plan reasonably covered the client API surface, but the suite is mocked and never exercises the real endpoint, so a server-side rejection would not have shown up in CI.

Tests

Four cases covering the validation bag, message-only, non-JSON, and empty bodies. The full suite passes (60 tests). phpstan reports the same 106 pre-existing errors before and after, none of them in this file.

@maliknikolaj

maliknikolaj commented Sep 3, 2026 •

Copy link
Copy Markdown

We hit the same thing this morning, and I think I found what's actually behind the empty 4xx, so the message fix here is treating the symptom.

1.70.4 allows Guzzle 8, which pulls in guzzlehttp/psr7 3. psr7 3 preserves the HTTP method casing (Request::__construct no longer uppercases it - see "Preserve request method casing" in the 3.0.0 notes), while ConsoleVaporClient passes methods in lowercase everywhere (requestWithErrorHandling('post', ...), request('get', ...) and so on). So with Guzzle 8 the CLI literally sends post /api/... HTTP/1.1, and the ELB in front of vapor.laravel.com answers that with an HTML 400 Bad Request before the request ever reaches the app. Easy to reproduce without a token:

curl -s -o /dev/null -w '%{http_code}\n' -X POST https://vapor.laravel.com/api/projects/1/environments/production/linted-manifest   # 401
curl -s -o /dev/null -w '%{http_code}\n' -X post https://vapor.laravel.com/api/projects/1/environments/production/linted-manifest   # 400, text/html

That's why linted-manifest is the first thing to die and why the body has no validation bag. Pinning 1.70.3 (Guzzle 7) fixes deploys for us. For the CLI itself, uppercasing the method in requestWithoutErrorHandling() (or requiring guzzlehttp/psr7:<3) should be enough.

@abedshamia

Copy link
Copy Markdown

Confirming the same failure on our side, with the same timeline.

  • Our staging deploy via GitHub Actions (composer global require laravel/vapor-cli, unpinned, PHP 8.1 on ubuntu-latest) succeeded at 07:03 UTC today on v1.70.3.
  • v1.70.4 was published at 07:57 UTC. Every deploy from 08:55 UTC onward failed about 170 ms into vapor deploy, before any build step, with exactly the output described here: the Trying to access array offset on value of type null warning on ConsoleVaporClient.php:1541 followed by an empty "Whoops! There were some problems with your request."
  • Nothing in our repo changed between the good and failed runs (manifest identical, same workflow). Re-running the failed workflow reproduced it.
  • Pinning laravel/vapor-cli:1.70.3 in the workflow fixed it immediately.

So the Guzzle 8 / User-Agent observation matches what we see. Happy to test a build against our environment if that helps narrow it down on the API side.

@datashaman

Copy link
Copy Markdown
Contributor Author

You're right, and my note about the User-Agent was wrong — thanks for chasing it down properly. I verified your diagnosis end to end:

guzzlehttp/psr7 2.x uppercases in the Request constructor, 3.x doesn't:

// psr7 2.13.1
self::warnOnMethodCasingChange($method);
$this->method = Utils::asciiToUpper($method);

// psr7 3.1.0
$this->method = $method;

Capturing the request line against a local socket, calling it the way requestWithoutErrorHandling() does — ->request('post', ...) rather than ->post(...):

guzzle 7.15.5 / psr7 2.13.1   POST /api/projects/1/environments/production/linted-manifest HTTP/1.1
guzzle 8.1.0  / psr7 3.1.0    post /api/projects/1/environments/production/linted-manifest HTTP/1.1

And your curl reproduction against the real endpoint:

POST -> 401  application/json  {"message":"Unauthenticated."}
post -> 400  text/html         <html>...<h1>400 Bad Request</h1>...

For the record on how I got it wrong: my earlier capture used $client->post(...), which goes through ClientTrait::post() and passes an uppercase literal, so both versions looked identical on the wire and the User-Agent was the only thing left differing. Calling request() with the lowercase verb, as the CLI does, shows it immediately.

Worth adding that the choke point is narrow. All 88 lowercase call sites go through requestWithoutErrorHandling(), so strtoupper($method) there covers every one; AwsStorageProvider already uses 'PUT' uppercase in all four places, so the asset upload path is unaffected.

On this PR: agreed it's the symptom, not the cause, and it shouldn't be merged as a fix for the deploy breakage. I'd still argue it earns its place — with it, that 400 surfaces as

Whoops! There were some problems with your request.

    - <html> <head><title>400 Bad Request</title></head> <body> <center><h1>400 Bad Request</h1></center> </body> </html>

instead of a bare "Whoops!", which points straight at the edge rather than at the app, and is what would have made this a five-minute diagnosis for both of us. I've pushed a tweak to collapse whitespace so an HTML page reads on one line.

Happy to add the strtoupper() here if the maintainers would rather have one PR, or to stay out of the way if you're opening one — your find, so your call.

@abedshamia

Copy link
Copy Markdown

Following up: the lowercase-method explanation from @maliknikolaj matches our failure exactly, so please disregard the User-Agent line in my comment above. Reproduced from our side as well: curl -X post against the linted-manifest endpoint returns an HTML 400, -X POST does not. For anyone landing here, our deploys are green again on the laravel/vapor-cli:1.70.3 pin (Guzzle 7.15.5); we will drop the pin once the strtoupper fix ships.

@datashaman

Copy link
Copy Markdown
Contributor Author

Opened #286 with the strtoupper() fix, since nobody had raised it yet and there are now three of us pinned to 1.70.3. @maliknikolaj — the diagnosis is credited to you in the description; say the word and I'll close it in favour of your own.

To keep the two straight for whoever reviews:

They're independent, so #286 can go in on its own.

@maliknikolaj

Copy link
Copy Markdown

@datashaman No need to close anything, #286 is exactly the fix I'd have opened and it's in the right place. Thanks for picking it up and for the credit. Hopefully both land soon.

@taylorotwell

Copy link
Copy Markdown
Member

Thanks for your pull request to Laravel!

I appreciate you taking the time to submit this; however, it appears this contribution may have been primarily AI-generated without careful human review and consideration.

We've found that AI-generated code often doesn't align well with Laravel's conventions, architectural decisions, and the specific context of what we're trying to accomplish with the framework. Quality contributions require thoughtful human insight into the codebase.

If you're interested in contributing to Laravel, I'd encourage you to familiarize yourself with the existing codebase, engage with the community, and submit PRs that reflect your own understanding and careful consideration of the problem you're solving.

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.

4 participants