Fixed internal server error for query urlResolver #445

yogeshsuhagiya · 2019-03-06T11:01:33Z

Description (*)

If Custom URL Rewrite is not autogenerated means manually created from backend then try to find relative_url based on target path.
Case 1: If target path is correct then return the result.
Case 2: If target path is wrong, means there no category, product, and cms page exists then throws erros.

Fixed Issues (if relevant)

Internal server error for query urlResolver while message "We can't find products matching the selection." appears on frontend #436

Manual testing scenarios (*)

1. Create Custom URL Rewrite

Request Path: bla-bla-bla
Target Path: catalog/category/view/id/5
Redirect Type: No

2. Perform query:

query{
  urlResolver(
    url: "bla-bla-bla"
  ) {
    id
    relative_url
    type
  }
}

3. Result:

{
    "data": {
        "urlResolver": {
            "id": 5,
            "canonical_url": "catalog/category/view/id/5",
            "type": "CATEGORY"
        }
    }
}

Contribution checklist (*)

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

app/code/Magento/UrlRewriteGraphQl/Model/Resolver/EntityUrl.php

naydav · 2019-03-06T22:55:03Z

@yogeshsuhagiya
Also, need to check failed test
https://travis-ci.com/magento/graphql-ce/jobs/182696242

…sh-1 Changed canonical_url to relative_url

… rewrite rule

yogeshsuhagiya · 2019-03-14T04:47:00Z

Hi @naydav , can you help with the following issue?

log: 
✓ SIGNIFYD_GLOBAL object initialization check
 jsbuild module
   - caches original load method...X caches original load method
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (1)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (2)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (3)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (4)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (5)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (6)
   - loads external files...
Warning: PhantomJS timed out, possibly due to an unfinished async spec. Use --force to continue.
Aborted due to warnings.

yogeshsuhagiya · 2019-03-15T04:50:45Z

Hi @naydav, can you please review this PR?

yogeshsuhagiya · 2019-04-02T05:28:44Z

Hi @naydav, can you please review this PR?

naydav · 2019-04-03T03:21:43Z

@galaoleksandr
Please, verify this PR from QA side
Thanks

naydav · 2019-04-03T03:31:43Z

Hi @naydav , can you help with the following issue?

log: 
✓ SIGNIFYD_GLOBAL object initialization check
 jsbuild module
   - caches original load method...X caches original load method
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (1)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (2)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (3)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (4)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (5)
     Error: Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL. (6)
   - loads external files...
Warning: PhantomJS timed out, possibly due to an unfinished async spec. Use --force to continue.
Aborted due to warnings.

Hi @yogeshsuhagiya
It was just random fail on Travis

Sorry for a delay, it is because that current milestone is devoted to Checkout workflow and Performance Improvement. But all of the PRs will be processed.

…aphql-PR-yogesh-2

dev/tests/api-functional/testsuite/Magento/GraphQl/UrlRewrite/UrlResolverTest.php

galaoleksandr · 2019-04-04T06:33:30Z

@yogeshsuhagiya Hello. Tried to use wrong request path, but no error message appeared.

yogeshsuhagiya · 2019-04-04T06:43:07Z

Hi @galaoleksandr, currently I'm writing the Test Case. I'm sure you'll definitely face this message.

TomashKhamlai · 2019-04-23T14:26:23Z

app/code/Magento/UrlRewriteGraphQl/Model/Resolver/EntityUrl.php

@@ -73,6 +74,11 @@ public function resolve(
        $url = $customUrl ?: $url;
        $urlRewrite = $this->findCanonicalUrl($url);
        if ($urlRewrite) {
+            if (!$urlRewrite->getEntityId()) {


We hide errors here.

Simple example:

Create Custom URL Rewrite
Request Path: go-to-admin
Target Path: /admin
Redirect Type: No

Perform query:

query{ urlResolver( url: "go-to-admin" ) { id relative_url type } }

For this case expected response in Internal Server Error which means

Exception #0 (LogicException): Front controller reached 100 router match iterations

but we simply got "No such entity found with matching URL key: go-to-admin"

Maybe it would be better to rely on Magento error handling then just check if our response is casting to true or false? @naydav?

Exception #0 (LogicException): Front controller reached 100 router match iterations
Looks like it's bug in Magento. expected result: 404 Page not found

Does everything else work?

TomashKhamlai · 2019-04-23T14:42:40Z

Generally it works for redirect type 'No'. But hides the error if problem occurred during request.

magento2#21580

TomashKhamlai · 2019-04-25T10:43:25Z

@yogeshaureatelabs, could you please update your fork.

You can do it easily by following these steps:

# check connected remotes
git remote -v

# add upstream remote if not configured. ("mainline" is just a name. The default remote is always called "origin")
git remote add mainline https://github.com/magento/graphql-ce.git

# download from the remote
git fetch mainline

# check the current active branch
git branch

# checkout to 2.3-develop of your fork
git checkout 2.3-develop

# merge changes from the upstream to your fork
git merge mainline/2.3-develop

# optionally push everything or just commit and go to next step
git push origin 2.3-develop

# checkout to the branch of your PR
git checkout 2.3-develop-graphql-PR-yogesh-2

# merge 2.3-develop to your branch
git merge 2.3-develop

# Update pull request. It will trigger travis tests automatically.
git push origin 2.3-develop-graphql-PR-yogesh-2

yogeshsuhagiya · 2019-04-25T12:52:24Z

@TomashKhamlai Sure

yogeshsuhagiya · 2019-04-25T12:56:41Z

Hi @TomashKhamlai
I've followed the sequence of git command which you had given and updated the 2.3-develop-graphql-PR-yogesh-2 branch.

yogeshsuhagiya · 2019-04-26T04:59:22Z

Hi @TomashKhamlai, @naydav will anyone please let me know how much more time needs to process this PR because approx 50 days have been already passed but still waiting.

TomashKhamlai · 2019-04-26T09:11:45Z

@yogeshsuhagiya, sorry for the delays. I will try to explain what is happening now and how it has influence on PR processing.

Magento follows practices for backward compatible development. This requires from developers to concentrate all the effort on little component and avoid mistakes that will be hard or even impossible to fix after release. So there is a difference between magento2(deliver fixes as soon as possible, improve the existing functionality) and graphql-ce(create functionality, do less but do best and cover everything with tests like a mad :) ) PR processing.

I haven't seen any roadmap documented (doesn't mean that it doesn't exist) but now the main focus is around the checkout process and bug fixing. As far as I know next prioritized PRs will be related to My Account activity. For keeping track on the Community activity the best option is to join the weekly #graphql synс-up that is held on Tuesdays and participate in Slack channel. You can get more information on our Magento Community Engineering Slack #graphql

We appreciate your work and your patience. You are not alone:
#392 (comment)
#500 (comment)

TomashKhamlai · 2019-05-06T15:54:55Z

@naydav, I mentioned about error-masking. If it is allowed, I would accept this PR. From my point of view it would be better even to mask this issue magento/magento2#21580 too.
#625 is still open.

Functionally works. Negative testing was performed

…aphql-PR-yogesh-2

ghost · 2019-05-10T01:21:51Z

Hi @yogeshsuhagiya, 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.

Fixed internal server error for query urlResolver

ad4efb8

naydav added Progress: accept Progress: needs update and removed Progress: accept labels Mar 6, 2019

naydav suggested changes Mar 6, 2019

View reviewed changes

app/code/Magento/UrlRewriteGraphQl/Model/Resolver/EntityUrl.php Show resolved Hide resolved

Merge pull request #1 from yogeshsuhagiya/2.3-develop-graphql-PR-yoge…

57d8908

…sh-1 Changed canonical_url to relative_url

naydav added in-progress and removed Progress: needs update labels Mar 8, 2019

Added testGetNonExistentUrlRewrite Test case to cover custom type URL…

8b39ecc

… rewrite rule

Merge remote-tracking branch 'origin/2.3-develop' into 2.3-develop-gr…

273ea48

…aphql-PR-yogesh-2

naydav suggested changes Apr 3, 2019

View reviewed changes

dev/tests/api-functional/testsuite/Magento/GraphQl/UrlRewrite/UrlResolverTest.php Show resolved Hide resolved

naydav added Progress: needs update and removed in-progress labels Apr 3, 2019

Covered testNonExistentEntityUrlRewrite case

43ba611

galaoleksandr added QA failed and removed QA failed labels Apr 5, 2019

yogeshsuhagiya requested a review from naydav April 8, 2019 10:09

TomashKhamlai added Progress: ready for review and removed Progress: needs update labels Apr 9, 2019

TomashKhamlai added the QA in progress We are checking label Apr 23, 2019

TomashKhamlai reviewed Apr 23, 2019

View reviewed changes

TomashKhamlai added QA failed and removed QA in progress We are checking labels Apr 23, 2019

TomashKhamlai mentioned this pull request Apr 24, 2019

Error handling in GraphQL? #625

Closed

TomashKhamlai added the Progress: needs update label Apr 25, 2019

Merge branch '2.3-develop' into 2.3-develop-graphql-PR-yogesh-2

96c1e2c

TomashKhamlai added QA passed and removed Progress: needs update QA failed labels May 6, 2019

naydav added Progress: accept and removed Progress: ready for review labels May 6, 2019

magento-engcom-team assigned naydav May 7, 2019

magento-engcom-team added Release Line: 2.3 Component: UrlRewriteGraphQl labels May 7, 2019

Merge remote-tracking branch 'origin/2.3-develop' into 2.3-develop-gr…

ec2ee49

…aphql-PR-yogesh-2

naydav approved these changes May 8, 2019

View reviewed changes

magento-engcom-team merged commit ec2ee49 into magento:2.3-develop May 10, 2019

magento-engcom-team pushed a commit that referenced this pull request May 10, 2019

ENGCOM-5081: Fixed internal server error for query urlResolver #445

7a231da

magento-engcom-team added Award: bug fix Award: test coverage labels May 10, 2019

naydav mentioned this pull request May 10, 2019

Internal server error for query urlResolver while message "We can't find products matching the selection." appears on frontend #436

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed internal server error for query urlResolver #445

Fixed internal server error for query urlResolver #445

yogeshsuhagiya commented Mar 6, 2019 •

edited by magesanjay

naydav commented Mar 6, 2019

yogeshsuhagiya commented Mar 14, 2019

yogeshsuhagiya commented Mar 15, 2019

yogeshsuhagiya commented Apr 2, 2019

naydav commented Apr 3, 2019

naydav commented Apr 3, 2019

galaoleksandr commented Apr 4, 2019 •

edited

yogeshsuhagiya commented Apr 4, 2019

TomashKhamlai Apr 23, 2019 •

edited

TomashKhamlai Apr 23, 2019

naydav May 3, 2019

TomashKhamlai commented Apr 23, 2019 •

edited

TomashKhamlai commented Apr 25, 2019 •

edited

yogeshsuhagiya commented Apr 25, 2019

yogeshsuhagiya commented Apr 25, 2019

yogeshsuhagiya commented Apr 26, 2019

TomashKhamlai commented Apr 26, 2019

TomashKhamlai commented May 6, 2019

ghost commented May 10, 2019

Fixed internal server error for query urlResolver #445

Fixed internal server error for query urlResolver #445

Conversation

yogeshsuhagiya commented Mar 6, 2019 • edited by magesanjay

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

naydav commented Mar 6, 2019

yogeshsuhagiya commented Mar 14, 2019

yogeshsuhagiya commented Mar 15, 2019

yogeshsuhagiya commented Apr 2, 2019

naydav commented Apr 3, 2019

naydav commented Apr 3, 2019

galaoleksandr commented Apr 4, 2019 • edited

yogeshsuhagiya commented Apr 4, 2019

TomashKhamlai Apr 23, 2019 • edited

Choose a reason for hiding this comment

TomashKhamlai Apr 23, 2019

Choose a reason for hiding this comment

naydav May 3, 2019

Choose a reason for hiding this comment

TomashKhamlai commented Apr 23, 2019 • edited

TomashKhamlai commented Apr 25, 2019 • edited

yogeshsuhagiya commented Apr 25, 2019

yogeshsuhagiya commented Apr 25, 2019

yogeshsuhagiya commented Apr 26, 2019

TomashKhamlai commented Apr 26, 2019

TomashKhamlai commented May 6, 2019

ghost commented May 10, 2019

yogeshsuhagiya commented Mar 6, 2019 •

edited by magesanjay

galaoleksandr commented Apr 4, 2019 •

edited

TomashKhamlai Apr 23, 2019 •

edited

TomashKhamlai commented Apr 23, 2019 •

edited

TomashKhamlai commented Apr 25, 2019 •

edited