feat(signup): throttle user account creation #619
Conversation
m90
force-pushed
the
fr/signup-throttle
branch
11 times, most recently
from
f354047 to
243dd46
Compare
m90
force-pushed
the
fr/signup-throttle
branch
3 times, most recently
from
15f3808 to
20a6303
Compare
|
|
||
| class ThrottleSignup | ||
| { | ||
| public function handle(Request $request, Closure $next, string $limit, string $range) |
There was a problem hiding this comment.
Not sure if I'm doing something wrong here, but I couldn't convince Laravel to accept extra arguments of different types: they either have to be all string or all int (when ideally this would be int $limit, string $range).
There was a problem hiding this comment.
This limitation might have something to do with variadic functions.
It is also possible to add a type declaration before the ... token. If this is present, then all arguments captured by ... must match that parameter type.
from https://www.php.net/manual/en/functions.arguments.php#functions.variable-arg-list
| Log::warning("WARN_SIGNUP_THROTTLED: Given limit of '$limit' in range '$range' was exceeded, attempted account creation was blocked."); | ||
| return response()->json( | ||
| ["errors" => "Due to high load, we're currently not able to create an account for you. Please try again tomorrow or reach out through our contact page."], | ||
| 429 |
There was a problem hiding this comment.
Not sure if a 429 is super correct here, but everything else 4xx fits even worse https://developer.mozilla.org/en-US/docs/Web/HTTP/Status#client_error_responses
There was a problem hiding this comment.
Maybe 503 would be a better choice as it's not really the client's fault.
There was a problem hiding this comment.
I also think 503 might be more fitting
There was a problem hiding this comment.
Yeah, I'll sneak this in still before merging.
|
|
||
| class ThrottleSignup | ||
| { | ||
| public function handle(Request $request, Closure $next, string $limit, string $range) |
There was a problem hiding this comment.
This limitation might have something to do with variadic functions.
It is also possible to add a type declaration before the ... token. If this is present, then all arguments captured by ... must match that parameter type.
from https://www.php.net/manual/en/functions.arguments.php#functions.variable-arg-list
| Log::warning("WARN_SIGNUP_THROTTLED: Given limit of '$limit' in range '$range' was exceeded, attempted account creation was blocked."); | ||
| return response()->json( | ||
| ["errors" => "Due to high load, we're currently not able to create an account for you. Please try again tomorrow or reach out through our contact page."], | ||
| 429 |
There was a problem hiding this comment.
I also think 503 might be more fitting
Ticket https://phabricator.wikimedia.org/T335963
Requires wmde/wbaas-deploy#983 wbstack/charts#121 wbstack/ui#700