packbackbooks / lti-1-3-php-library

A library used for building IMS-certified LTI 1.3 tool providers in PHP.
Apache License 2.0
39 stars 23 forks source link

Logging Guzzle Exceptions / Adding previous to the exceptions #145

Closed Lokilein closed 1 month ago

Lokilein commented 1 month ago

We currently have a problem, that e.g. LTIMessageLaunch::getPublicKey() fails in the method ILtiServiceConnector::makeRequest. In its implementation, a debugLog is supported:

    public function makeRequest(IServiceRequest $request)
    {
        $response = $this->client->request(
            $request->getMethod(),
            $request->getUrl(),
            $request->getPayload()
        );

        if ($this->debuggingMode) {
            $this->logRequest(
                $request,
                $this->getResponseHeaders($response),
                $this->getResponseBody($response)
            );
        }

        return $response;
    }

However, when $this->client->request() throws an Exception, you will not get that debug log. Also, it is catched like this:

        try {
            $response = $this->serviceConnector->makeRequest($request);
        } catch (TransferException $e) {
            throw new LtiException(static::ERR_NO_PUBLIC_KEY);
        }

I think it would be great if either the exception is thrown with new LtiException(static::ERR_NO_PUBLIC_KEY, previous: $e) or the debug mode is able to log even when an exception is thrown, so the users of this library are able to find the underlying issue of this exception in some way.

dbhynds commented 1 month ago

@Lokilein Thanks for the recommendation. Can you look at the pull request associated with this issue and let me know if that's what you had in mind?

Lokilein commented 1 month ago

@dbhynds Yes, perfect! Thank you for the quick change!