Remove dep with symfony/http-kernel#135
Remove dep with symfony/http-kernel#135jissereitsma wants to merge 1 commit intomagewirephp:mainfrom jissereitsma:patch-1
Conversation
| $request = $this->httpFactory->createRequest($this->request->getParams())->isSubsequent(true); | ||
| } catch (LocalizedException $exception) { | ||
| throw new HttpException(400); | ||
| throw new RuntimeException(400); |
There was a problem hiding this comment.
I'm not sure this should be merged into the core. \RuntimeException's first arg requires a message and the second a exception code. The reason having the HttpException is not having to pass in each status message for simple 400, 404, 503 etc. Those are all default and will be rendered in the frontend.
Also, having the right status code is also important in the frontend.
Is there something I'm overlooking?
There was a problem hiding this comment.
No, you spotted a mistake I made. Having the HTTP status code available is indeed a requirement I missed. However, adding a composer package for this seems a bit much. One option could be to simply create a duplicate HttpException class in the magewire package (and then drop the dependency with symfony/http-kernel). Another option could be to simply refactor the code in the controller a bit further - the HttpException class is used only once as an argument to throwException($exception). The HTTP status code could easily be turned into an optional second argument (throwException($exception, $statusCode = null). Which option do you think is better? Happy to refactor this PR.
2 participants
As mentioned in #107 there is an undocumented dependency with
symfony/http-kernelonly for an exception that is thrown. TheHttpException(https://github.com/symfony/http-kernel/blob/6.3/Exception/HttpException.php) actually inherits from\RuntimeException. This PR simply just removes the dep and throws theRuntimeExceptionas is. Nothing fancy.Additional comment, the reason why this issue got started is that the
symfony/http-foundationpackage (which is a requirement for Magewire) has arequire-devdep withsymfony/http-kernel(https://github.com/symfony/http-foundation/blob/6.3/composer.json#L29). If composer is installed in dev env, then this problem does not occur.