Skip to content
This repository has been archived by the owner on Dec 19, 2019. It is now read-only.

Error handling in GraphQL? #625

Closed
TomashKhamlai opened this issue Apr 24, 2019 · 1 comment
Closed

Error handling in GraphQL? #625

TomashKhamlai opened this issue Apr 24, 2019 · 1 comment
Assignees
Labels
question Further information is requested

Comments

@TomashKhamlai
Copy link
Contributor

TomashKhamlai commented Apr 24, 2019

As a QA specialist I want to know the design of error handling in GraphQL. It looks like we miss this in our Wiki.

The questions are:

  1. All error messages or just last one? What are the expectations of developers that will build apps for Magento2?
    I believe that Magento is safe to provide all errors that happened during request an exception could be only Internal Server Error that can be replaced with "Something bad has happened on our side". But doing in this way the behavior is not like on the frontend area. We often see only first(last) error. (look at graphql-input provides to Customer as many errors as appeared instead of last one like on Magento Storefront #416)

  2. What about aggregated errors? Case Error message for mutation generateCustomerToken #628:

  • Generate Customer Token without credentials
    For REST API we have the following Class lib/internal/Magento/Framework/Oauth/Helper/Request.php which provides
    /**
     * Create response string for problem during request and set HTTP error code
     *
     * @param \Exception $exception
     * @param \Magento\Framework\HTTP\PhpEnvironment\Response $response OPTIONAL If NULL - will use internal getter
     * @return array
     */
    public function prepareErrorResponse(
        \Exception $exception,
        \Magento\Framework\HTTP\PhpEnvironment\Response $response = null
    ) {
        $errorMsg = $exception->getMessage();

        if ($exception instanceof \Magento\Framework\Oauth\Exception) {
            $responseCode = self::HTTP_UNAUTHORIZED;
        } elseif ($exception instanceof \Magento\Framework\Oauth\OauthInputException) {
            $responseCode = self::HTTP_BAD_REQUEST;
            if ($errorMsg == 'One or more input exceptions have occurred.') {
                $errorMsg = $exception->getAggregatedErrorMessage();
            }
        } else {
            $errorMsg = 'internal_error&message=' . ($errorMsg ? $errorMsg : 'empty_message');
            $responseCode = self::HTTP_INTERNAL_ERROR;
        }

        $response->setHttpResponseCode($responseCode);
        return ['oauth_problem' => $errorMsg];
    }

and the expectations are in dev/tests/api-functional/testsuite/Magento/Integration/Model/CustomerTokenServiceTest.php

    /**
     * Assert for presence of Input exception messages
     *
     * @param \Exception $e
     */
    private function assertInputExceptionMessages($e)
    {
        $this->assertEquals(HTTPExceptionCodes::HTTP_BAD_REQUEST, $e->getCode());
        $exceptionData = $this->processRestExceptionResult($e);
        $expectedExceptionData = [
            'message' => 'One or more input exceptions have occurred.',
            'errors' => [
                [
                    'message' => '"%fieldName" is required. Enter and try again.',
                    'parameters' => [
                        'fieldName' => 'username',
                    ],
                ],
                [
                    'message' => '"%fieldName" is required. Enter and try again.',
                    'parameters' => [
                        'fieldName' => 'password',
                    ]
                ],
            ],
        ];
        $this->assertEquals($expectedExceptionData, $exceptionData);
    }

GraphQL client receives only:

{
   "data" : {
      "generateCustomerToken" : null
   },
   "errors" : [
      {
         "locations" : [
            {
               "column" : 84,
               "line" : 1
            }
         ],
         "debugMessage" : "One or more input exceptions have occurred.",
         "message" : "Internal server error",
         "path" : [
            "generateCustomerToken"
         ],
         "category" : "internal"
      }
   ]
}
  1. Do we take errors from config like it was suggested in 526#discussion_r268233903 or generate our own?
  2. Do we allow error-masking if the error message is generated by GraphQL module?
    described in 445#discussion_r277706576
  3. So everything I know is that errors should be provided along with data by design. (errors[] and data[] both in the same response #379)

Expected result:

  1. Guidelines or Checklist
@lenaorobei
Copy link
Contributor

Closed in favor of #970

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants