BUGFIX: PR 4425 FusionGlobals followup / fix
#4527
Conversation
We don't want first level support for this!
Configures this runtime to override the default exception handler configured in the settings or via Fusion's @ExceptionHandler {@see AbstractRenderingExceptionHandler}. In combination with the throwing handler {@see ThrowingHandler} this can be used to rethrow all exceptions. This is helpfully for renderings in CLI context or testing.
The Fusion RuntimeException declared a few optional parameters, which are actually not optional and code will fail if they return null. In the case for the Exceptions like 'Overriding Fusion global variable "request" via @context is not allowed.' - One should rather use the generic Fusion Exception.
| /** | ||
| * Configures this runtime to override the default exception handler configured in the settings | ||
| * or via Fusion's \@exceptionHandler {@see AbstractRenderingExceptionHandler}. | ||
| * | ||
| * In combination with the throwing handler {@see ThrowingHandler} this can be used to rethrow all exceptions. | ||
| * This is helpfully for renderings in CLI context or testing. | ||
| */ | ||
| public function overrideExceptionHandler(AbstractRenderingExceptionHandler $exceptionHandler): void | ||
| { | ||
| $this->overriddenExceptionHandler = $exceptionHandler; |
There was a problem hiding this comment.
This might need to be discussed - but i like this idea.
There was a problem hiding this comment.
@mficzel are you fine with this idea as well?
There was a problem hiding this comment.
Fine yes but i wonder why it is not mentioned in the description.
There was a problem hiding this comment.
added ;) lets merge? ^^
| * @param null $fusionPath | ||
| */ | ||
| public function __construct($message = '', $code = 0, \Exception $previous = null, $fusionPath = null) | ||
| public function __construct(string $message, int $code, \Exception $previous, string $fusionPath) |
There was a problem hiding this comment.
Is the api change to make $fusionPath non optional really intended. It makes sense but it is a little breaky.
There was a problem hiding this comment.
However i could not find any code that actually is harmed by this even in the oldest projects i found. So it is probably safe
There was a problem hiding this comment.
au contraire, not changing it will allow the fusion path at first to be empty and result in an obscure null error as we trust this value at other places to be non null -> thats why i changed it to avoid future pitfalls of us fusion runtime developers, and make it crystal clear that it is not nullable!
And i also consider the runtime exception to be internal api ...
There was a problem hiding this comment.
THANK you for making sure i dont break stuff again btw ;) ❤️
mhsdesign
deleted the
feature/customDefaultContextVariablesFusionRuntime-followup
branch
This is a little followup to #4425
This fixes a bug with the error handling when overriding a fusion global variable.
Also we add a possibility to override the exception handler in the Runtime.
It configures this runtime to override the default exception handler configured in the settings
or via Fusion's
@exceptionHandler.In combination with the throwing handler (
ThrowingHandler) this can be used to rethrow all exceptions.This is helpfully for renderings in CLI context or testing.
Upgrade instructions
Review instructions
Best reviewed commit by commit ;)
In case one overrides a fusion global an error should occur. Currently there is an error in the error handling:
This was caused by using the RuntimeException wrongly
Checklist
FEATURE|TASK|BUGFIX!!!and have upgrade-instructions