[11.x] Improve API error handling

[tontonsb]

I've come across two common issues when working with API backends and testing the endpoints in browser or just not knowing to specify the Accept: application/json header. Most projects work around this by adding some middleware that either adds the header to all incoming API requests or sets a forceJson flag on their custom request.

This PR proposes to remedy these issues by taking a "only redirect if there's a reasonable redirect target" approach.

Unauthenticated user

When hitting an endpoint that requires authentication, but failing the auth, user should receive an authentication error. Instead they receive a 500 error with Route [login] not defined. message.

Proposed solution: only redirect to route('login') if the route exists. Otherwise fall back to JSON response.

Backwards compatibility? Technically it is a breaking change, but I can't imagine a legit scenario where someone would rely on their auth errors being redirected to non-existant route, catching the particular error and doing some useful reporting with it.

Validation error

When getting some request params wrong, users should see validation errors. Instead users that are hitting the endpoint in their browserse receive a redirect to /. Because no "previous" route exists and even if it did, Laravel wouldn't know it as the session is not enabled on API routes.

Proposed solution: only redirect to "previous" route if it exists in the session. Do not use url()->previous() as

• falling back to url('/') for all validation errors is unlikely to be useful in real usecases (although common in automated tests)
• redirecting to referrer would be useless as they wouldn't receive the error bag or error code in any way

Backwards compatibility?

1. This will require to update web (non-json) validation tests that expect a 302 redirect now. Two options:
   • Expect 422 instead of 302, i.e. change assertRedirect or assertStatus(302) to assertInvalid or assertStatus(422).
   • Set up the session with the previous URL as it would be in a real life workflow. Add session()->setPreviousUrl($prevUrl); to the test before hitting the endpoint that should redirect you back.
2. This would also break the workflow if someone has set their index page as a dedicated error displayer. I don't know if someone actually does that. There are multiple ways to restore the previous behaviour, e.g. overriding the handler's invalid method or catching the exceptions in handler (and maybe just specifying ->redirectTo('/') on them which is still respected).

[ankurk91]

I faced the same issue recently, since my app did not have any route named login.

Laravel core must not auto assume that the this route exists in Skelton

[joelbutcher]

When would an application not have a login route, but yet still have routes protected by auth that would throw an authentication exception like this?

[tontonsb]

When you're using any other auth mechanism, e.g. token in the headers or query.

[joelbutcher]

Correct me if I'm wrong, but with this logic it is currently possible for a route to return a JSON response when it's not expected to, if there is no $redirectTo target. Is this correct behaviour?

[tontonsb]

Yes, that's exactly the point of this change.

I am arguing that receiving 500 | Server error or Route [login] not defined. is wrong (and is not useful) if the actual issue is that you're not authenticated and returning the JSON message is the most reasonable fallback in that case.

[taylorotwell]

Going to revisit this a bit later. I'm currently doing some heavy work on the skeleton, exception handler, etc.