in2code-de / luxletter

Newsletter system for TYPO3
https://www.in2code.de/agentur/typo3-extensions/luxletter/
22 stars 25 forks source link

[BUGFIX] Passthrough RequestException #222

Closed julianhofmann closed 2 months ago

julianhofmann commented 2 months ago

If a RequestException is thrown, then this should also be passed on instead of (incorrectly) relabeling it as a MisconfigurationException and throwing it.

Related: #221

einpraegsam commented 2 months ago

Thx for your suggest. I just checked your requested change. First of all, I think you are basically right. We should pass more information if there are errors. But instead of throwing another exception, I would tend to extend the existing errormessage by adding the original message at the end. What do you think?

julianhofmann commented 2 months ago

Initially, I tend to do exactly this. Our catch-block is throwing a MisconfigurationException - that's different from a RequestException. I would interpret "misconfiguration" to be something in TypoScript, a fault, an integrator has made. A RequestException can have different reasons - and might not be fixable by the integrator but needs more technical effort or support by server guys. So the handling of these both exceptions might be different.

einpraegsam commented 2 months ago

MisconfigurationException is maybe not correct. It should be more a RequestFailureException in my eyes

julianhofmann commented 2 months ago

Different reasons for exceptions, so maybe different handling. I therefore think it is wrong to summarize it in one exception - regardless of whether it is of one or the other type of exception.

einpraegsam commented 2 months ago

Allright, do you have a screenshot for me how the user sees the request exception? I want to get a feeling how helpful this change is.

julianhofmann commented 2 months ago

Oh, maybe I understand the confusion and the questions... I had caught the wrong RequestException during import, but meant the right one as a solution :-/

The accidentally caught use In2code\Luxletter\Exception\RequestException; brings no advantage because it does not bring any information with it. I wanted to handle the GuzzleHttp\Exception\RequestException


Requested example

Let's imagine that our TYPO3 is access-protected. In a simple way, e.g. with this .htaccess:

AuthType Basic
AuthName "NO PUBLIC ACCESS"
AuthUserFile /xxx/.htpasswd

SetEnvIf REQUEST_URI "/typo3/" ALLOW
SetEnvIf REQUEST_URI "/_assets/" ALLOW

<RequireAny>
  Require env ALLOW
  Require valid-user
</RequireAny>

Now we want to create a new newsletter via scheduler task (command: luxletter:createnewsletterfromorigin) and start the task from the scheduler backend module:

einpraegsam commented 2 months ago

Yes, that seems to me helpful