Skip to content
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

[5.5] Throttles request - throw a 429 HTTP exception. #19860

Merged
merged 2 commits into from
Jul 4, 2017
Merged

[5.5] Throttles request - throw a 429 HTTP exception. #19860

merged 2 commits into from
Jul 4, 2017

Conversation

lucasmichot
Copy link
Contributor

@lucasmichot lucasmichot commented Jul 2, 2017

Directly call throw a 429 HTTP Exception, so that :

  • we let the exception handler return the response with a different content type
  • we can decide to log or not this kind of exception

@taylorotwell
Copy link
Member

Does an HTTP response still get passed back to previous middleware?

@lucasmichot
Copy link
Contributor Author

lucasmichot commented Jul 3, 2017

If an exception is thrown here, of course the request lifecycle stops here.
So the next middleware are not executed (like for any other HTTP exceptions BTW, 404 and so on...)

@taylorotwell
Copy link
Member

The previous middleware.

@lucasmichot
Copy link
Contributor Author

lucasmichot commented Jul 3, 2017

Yes. For instance, if the previous middleware \Illuminate\Auth\Middleware\Authenticate comes just before and you cannot access this endpoint, this means that you will just have a \Illuminate\Auth\AuthenticationException (without X-RateLimit-* headers).

But if you are authenticated and pass \Illuminate\Auth\Middleware\Authenticate, then \Illuminate\Auth\AuthenticationException will handle the previous response, then the exception will be thrown.

The way it works is exactly as in \Illuminate\Auth\Middleware\Authenticate::authenticate

@taylorotwell taylorotwell merged commit 45bfce0 into laravel:master Jul 4, 2017
@lucasmichot lucasmichot deleted the feature/master/throw-429-http-exception branch July 4, 2017 12:51
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.

2 participants