GraphQl-220: Implement exception logging

[novikor]

Description (*)

Created logging for client and server errors.
I use exception category from \GraphQL\Error\ClientAware to divide errors between the client (var/log/graphql/client/exception.log) and server (var/log/graphql/server/exception.log).
In case the category is not recognized, the errors will be logged into a general log (var/log/graphql/exception.log)

Fixed Issues (if relevant)

1. Implement exception logging #220: Implement exception logging

Manual testing scenarios (*)

1. Cause any client-side error (Incorrect query parameter, for example) and check var/log/graphql/client/exception.log
2. Manually throw exception elsewhere on backend during query processing, and check var/log/graphql/server/exception.log

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[magento-cicd2]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ vpodorozh
✅ novikor
❌ Maksym Novik

---

Maksym Novik seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

[novikor]

Probably, it could be a better idea to use \GraphQL\Error\ClientAware::isClientSafe to detect if it is a client-side error instead of \GraphQL\Error\ClientAware::getCategory.

It this case there will be no need to update the virtualtypes every time new exception category is created.

[novikor]

@naydav , @paliarush
I can suggest 2 ways to divide errors into client and server:

• Use \GraphQL\Error\ClientAware::isClientSafe
• Just check if the exception is an instance of \Magento\Framework\Exception\LocalizedException

[vpodorozh]

@magento run all tests

[vpodorozh]

@novikor - please check my review

[vpodorozh]

@naydav , @paliarush
I can suggest 2 ways to divide errors into client and server:

• Use \GraphQL\Error\ClientAware::isClientSafe
• Just check if the exception is an instance of \Magento\Framework\Exception\LocalizedException

@novikor - I've put the comments to review from @naydav about my vision.
I think we can start implementing it (my vision :) ) from Wednesday (19.06) in case guys will not respond with their notes.

Thank you!

Not actual anymore

[vpodorozh]

seems to be ok - tested it locally with different error types. Waiting for test builds.

[vpodorozh]

@novikor - please check my change request.

@swnsma - please pick up this PR as a reviewer, I do not have any permissions for approving.
Thx!

[novikor]

Yep. It will be finished this week.

[novikor]

PR is ready for review.

Integration or at least Unit tests will be provided at the end of the current week.

[TomashKhamlai]

@novikor, please try these steps:

1. php bin/magento deploy:mode:set developer
2. Perform steps described in [Customizable Options] Call to a member function format() on boolean #761

Expected result(until #761 is not fixed):

{
  "errors": [
    {
      "debugMessage": "Call to a member function format() on boolean",
      "message": "Internal server error",
      "category": "internal",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "addSimpleProductsToCart"
      ]
    }
  ],
  "data": {
    "addSimpleProductsToCart": null
  }
}

Actual result:

{
  "errors": [
    {
      "message": "Internal server error",
      "category": "internal",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "addSimpleProductsToCart"
      ]
    }
  ],
  "data": {
    "addSimpleProductsToCart": null
  }
}

It seems like a regression. Also, it doesn't work for me in production mode. I do not see any error log, do you see any?

[novikor]

Looks strange. I will check again

[novikor]

Hi, @TomashKhamlai .
I have rechecked your case and failed to reproduce the problem.
Please take my patch for simplifying steps to reproduce (attached).
GraphQl-220__Implement_exception_logging_.patch.zip

Use the next query:

{
  products(filter: {}) {
    total_count
  }
}

[lenaorobei]

I've tested your branch and it logs the same message in 2 places var/log/graphql/exception.log and exception.log. Please consider logging in one place, I would suggest var/log/exception.log.

[novikor]

Fixed

[ghost]

Hi @novikor, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.