Bug 169970 – JavaScript is not correctly connecting to rest api in iOS 10.3 beta

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: JavaScriptCore

[paladox]

Hi, since upgrading to iOS 10.3 beta loading websites like https://gerrit-review.googlesource.com?polygerrit=1 is no loading.

https://github.com/GerritCodeReview/gerrit/tree/master/polygerrit-ui

Also testing in safari technology preview results in the same issue.

I am unsure if it is a bug that was introduced by apple or if it is a bug in webkit. So I'm filling it here. I filled a report here https://bugs.chromium.org/p/gerrit/issues/detail?id=5715 for gerrit.

Sorry that i doint have much more information. I tried for a week to get more info but i carn't as the developer tools for js errors isn't telling me much.

iOS 10.2 is un affected.

[paladox]

Oh wait it seems to work on gerri-review with the safari technology preview so it seems it is just iOS 10.3.

[paladox]

Testing locally and this fails on safari technology preview but not on gerrit-review site. So maybe cross origin?

[paladox]

(In reply to paladox from comment #2)
> Testing locally and this fails on safari technology preview but not on
> gerrit-review site. So maybe cross origin?

I should i press the clear cache button. It works locally now with safari technology preview but not on iOS 10.3

[Simon Fraser (smfr)]

Does the Web Inspector consoles show any errors?

[paladox]

Hi, the only error it shows is this

console.error(e.detail.error.message);
             ^
Is showing as undefined not an object.

But removing that part stops the undefined not an object error in the console but it fix the problem.

[paladox]

^^ sorry where I say it fixed the problem, it dosent fix the problem.

[paladox]

Bum.

I am not 100% correct its about fetching json but what ever the problem is it is not correctly connecting to the rest api on gerrit using polygerrit. I doint see anything in the console to explain this problem. Polygerrit throws the network-error error internally to throw Server Unavilable.

This works on iOS 10.2 but breaks on iOS 10.3 beta.

[paladox]

bum = bump.

[Simon Fraser (smfr)]

Does the same problem occur on Mac, with built-in Safari or Safari Tech Preview?

[paladox]

Hi nope doesn't happen on safari or the safari technology preview. Im running mac OS X 10.12.3.

[paladox]

It looks like polygerrit uses https://github.com/github/fetch/

Example code

   fetchJSON: function(url, opt_errFn, opt_cancelCondition, opt_params,
        opt_opts) {
      opt_opts = opt_opts || {};
      var fetchOptions = {
        credentials: 'same-origin',
        headers: opt_opts.headers,
      };

      var urlWithParams = this._urlWithParams(url, opt_params);
      return fetch(urlWithParams, fetchOptions).then(function(response) {
        if (opt_cancelCondition && opt_cancelCondition()) {
          response.body.cancel();
          return;
        }

        if (!response.ok) {
          if (opt_errFn) {
            opt_errFn.call(null, response);
            return;
          }
          this.fire('server-error', {response: response});
          return;
        }

        return this.getResponseObject(response);
      }.bind(this)).catch(function(err) {
        if (opt_errFn) {
          opt_errFn.call(null, null, err);
        } else {
          this.fire('network-error', {error: err});
          throw err;
        }
        throw err;
      }.bind(this));
    },

[paladox]

It seems safari 10.1 includes native fetch api now https://developer.apple.com/library/prerelease/content/releasenotes/General/WhatsNewInSafari/Articles/Safari_10_1.html that could be breaking this.

[paladox]

Maybe safari 10.1 is missing the Fetch API: streaming response body api?

According to https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API it doesn't have that api in safari 10.1. I think polygerrit uses that api.

[paladox]

Nope polygerrit doesn't seem to use streaming response body api as the GitHub fetch library doesn't use it. So theres a bug some where in webkit with fetch.

[paladox]

It seems the response api in fetch is not in safari 10.1 https://developer.mozilla.org/en-US/docs/Web/API/Response and polygerrit uses that https://github.com/gerrit-review/gerrit/search?l=JavaScript&q=response.ok&type=&utf8=%E2%9C%93

[paladox]

Also according to https://developer.mozilla.org/en-US/docs/Web/API/Body/text the response.text is not in safari. But we read the json of it here https://github.com/GerritCodeReview/gerrit/blob/af3ec9a6df468fd049ea271db29ede0ccc8f8a3d/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js#L169

So that could also be the reason too.

[youenn fablet]

Hi paladox,

Are you seeing this stack trace (to get it, open web inspector and place a breakpoint where the error is raised)?

anonymous@[native code]
fillFetchHeaders@[native code]
initializeFetchRequest@[native code]
[native code]
fetch@[native code]
fetchJSON@https://cdn.googlesource.com/polygerrit_ui/304.0/elements/gr-app.js:9878:19
_fetchSharedCacheURL@https://cdn.googlesource.com/polygerrit_ui/304.0/elements/gr-app.js:10145:54
attached@https://cdn.googlesource.com/polygerrit_ui/304.0/elements/gr-app.js:40761:31

[youenn fablet]

If this is the stack trace you are seeing, the issue might be https://bugs.webkit.org/show_bug.cgi?id=168043.
You might want to verify this by checking the fetch options passed and, if you can, modify it to not have any option with {headers: undefined}.

[paladox]

(In reply to youenn fablet from comment #17)
> Hi paladox,
> 
> Are you seeing this stack trace (to get it, open web inspector and place a
> breakpoint where the error is raised)?
> 
> anonymous@[native code]
> fillFetchHeaders@[native code]
> initializeFetchRequest@[native code]
> [native code]
> fetch@[native code]
> fetchJSON@https://cdn.googlesource.com/polygerrit_ui/304.0/elements/gr-app.
> js:9878:19
> _fetchSharedCacheURL@https://cdn.googlesource.com/polygerrit_ui/304.0/
> elements/gr-app.js:10145:54
> attached@https://cdn.googlesource.com/polygerrit_ui/304.0/elements/gr-app.js:
> 40761:31

Hi, nope i doint see any trace back except from it thinks e.detail.error.message is undefined.

It seems you got the traceback.

I ran this

    var result = fetch('https://api.github.com')
    result.then(function(response) {
      console.error('response', response)
      console.error('header', response.headers.get('Content-Type'))
      return response.text()
    }).then(function(text) {
      console.error('got text', text)
    }).catch(function(ex) {
      console.error('failed', ex)
    })

through tried this on jsfiddle on iOS 10.3 and got

iOS 10.3

{
  \"current_user_url\": \"https://api.github.com/user\",
  \"current_user_authorizations_html_url\": \"https://github.com/settings/connections/applications{/client_id}\",
  \"authorizations_url\": \"https://api.github.com/authorizations\",
  \"code_search_url\": \"https://api.github.com/search/code?q={query}{&page,per_page,sort,order}\",
  \"commit_search_url\": \"https://api.github.com/search/commits?q={query}{&page,per_page,sort,order}\",
  \"emails_url\": \"https://api.github.com/user/emails\",
  \"emojis_url\": \"https://api.github.com/emojis\",
  \"events_url\": \"https://api.github.com/events\",
  \"feeds_url\": \"https://api.github.com/feeds\",
  \"followers_url\": \"https://api.github.com/user/followers\",
  \"following_url\": \"https://api.github.com/user/following{/target}\",
  \"gists_url\": \"https://api.github.com/gists{/gist_id}\",
  \"hub_url\": \"https://api.github.com/hub\",
  \"issue_search_url\": \"https://api.github.com/search/issues?q={query}{&page,per_page,sort,order}\",
  \"issues_url\": \"https://api.github.com/issues\",
  \"keys_url\": \"https://api.github.com/user/keys\",
  \"notifications_url\": \"https://api.github.com/notifications\",
  \"organization_repositories_url\": \"https://api.github.com/orgs/{org}/repos{?type,page,per_page,sort}\",
  \"organization_url\": \"https://api.github.com/orgs/{org}\",
  \"public_gists_url\": \"https://api.github.com/gists/public\",
  \"rate_limit_url\": \"https://api.github.com/rate_limit\",
  \"repository_url\": \"https://api.github.com/repos/{owner}/{repo}\",
  \"repository_search_url\": \"https://api.github.com/search/repositories?q={query}{&page,per_page,sort,order}\",
  \"current_user_repositories_url\": \"https://api.github.com/user/repos{?type,page,per_page,sort}\",
  \"starred_url\": \"https://api.github.com/user/starred{/owner}{/repo}\",
  \"starred_gists_url\": \"https://api.github.com/gists/starred\",
  \"team_url\": \"https://api.github.com/teams\",
  \"user_url\": \"https://api.github.com/users/{user}\",
  \"user_organizations_url\": \"https://api.github.com/user/orgs\",
  \"user_repositories_url\": \"https://api.github.com/users/{user}/repos{?type,page,per_page,sort}\",
  \"user_search_url\": \"https://api.github.com/search/users?q={query}{&page,per_page,sort,order}\"
}

[paladox]

Is there any script i could try to test weather the header being undefined is causing this please?

[paladox]

Doing something like



function _urlWithParams(url) {
      return url;
    }
    var urlWithParams = _urlWithParams('https://gerrit-review.googlesource.com/config/server/version');
var fetchOptions = {
        //headers: '',
        credentials: 'same-origin',
        mode: 'no-cors',
        headers: {'content-type': 'application/json'},
      };
  fetch(urlWithParams, fetchOptions)
    .then(function(response) {
      console.error('response', response)
      console.error('header', response.headers.get('Content-Type'))
      return response.text().then(function(text) {
        var result;
        try {
          result = JSON.parse(text.substring(JSON_PREFIX.length));
        } catch (_) {
          result = null;
        }
        console.error('got text', text)
        return result;
      });
    }).catch(function(ex) {
      console.error('failed', ex)
    })

fails. I may have done it wrong but not sure.

[paladox]

It doesn't respond with a url

response Response {type: "opaque", url: "", redirected: false, status: 0, ok: false…}

[paladox]

The traceback is leading me to this line


    fetchJSON: function(url, opt_errFn, opt_cancelCondition, opt_params,
        opt_opts) {
      opt_opts = opt_opts || {};
      var fetchOptions = {
        credentials: 'same-origin',
        headers: opt_opts.headers,
      };

      var urlWithParams = this._urlWithParams(url, opt_params);
      return fetch(urlWithParams, fetchOptions).then(function(response) {
        if (opt_cancelCondition && opt_cancelCondition()) {
          response.body.cancel();
          return;
        }

        if (!response.ok) {
          if (opt_errFn) {
            opt_errFn.call(null, response);
            return;
          }
          this.fire('server-error', {response: response});
          return;
        }

        return this.getResponseObject(response);
      }.bind(this)).catch(function(err) {
        if (opt_errFn) {
          opt_errFn.call(null, null, err);
        } else {
          this.fire('network-error', {error: err});
          throw err;
        }
        throw err;
      }.bind(this));
    },

[paladox]

I added console.error to fetchJson and i got some logs in the console if i used safari on the mac or the safari technology preview. But i got none on iOS 10.3 safari.

[youenn fablet]

Can you update fetchOptions so that its 'headers' value has a real value or does not exist at all. Having {headers: undefined} might be the issue.
Also you can get the stack trace doing something like e.detail.error.stack

[paladox]

I found safari technology preview 23 download. I tried that and polygerrit docent work in that release. It seems indeed its the undefined header thing. Could your fix be back ported into safari 10.1 for iOS 10.3 and presumingly mac 10.12.4 is using it, please?

[paladox]

(In reply to youenn fablet from comment #25)
> Can you update fetchOptions so that its 'headers' value has a real value or
> does not exist at all. Having {headers: undefined} might be the issue.
> Also you can get the stack trace doing something like e.detail.error.stack

I doint know how to get the header to be anything else. I tried crossing out headers but that still didn't work.

[paladox]

Removing headers: seems to work on the safari technology preview 23.

[paladox]

It also works on ios when removing the headers part.

[paladox]

Removing the headers: seems to break refreshing a diff. So could the fix you did be back ported to 10.1 please?

[youenn fablet]

Can you try setting headers only if not undefined?
Something like:
fetchOptions ={}
if (opt_opts.headers)
    fetchOptions.headers = opt_opts.headers

[paladox]

I did 

      if (opt_opts.headers !== undefined) {
      var fetchOptions = {
        credentials: 'same-origin',
        headers: opt_opts.headers,
      };
      } else {
      var fetchOptions = {
        credentials: 'same-origin',
       // headers: opt_opts.headers,
      };
      }

which is the same as if(opt_opts.headers).

But still doesn't work in reloading the diff page. For example going to http://gerrit-new.wmflabs.org/r/c/2/62/tests/fixtures/layout-cloner.yaml and then reloading that downloads a json file (the fix i did for headers is not on that site). It should not be downloading json files when refreshing the page. Which i presume thats why headers: is there to fix that.

[paladox]

the diff loads if you click on the file to view the diff http://gerrit-new.wmflabs.org/r/c/2/

for example ^^ going to that and clicking on the file works. But refreshing it downloads a json file instead of reloading the page. Same for when clicking the next and prev buttons on the diff.

[paladox]

@youenn fablet hi, is there a way i can remove the headers: part only on the browser versions that the problem exist. Like safari 10.1 please? Since iOS 10.3 and mac os 10.12.4 have rolled out now.

[youenn fablet]

(In reply to paladox from comment #32)
> I did 
> 
>       if (opt_opts.headers !== undefined) {
>       var fetchOptions = {
>         credentials: 'same-origin',
>         headers: opt_opts.headers,
>       };
>       } else {
>       var fetchOptions = {
>         credentials: 'same-origin',
>        // headers: opt_opts.headers,
>       };
>       }
> 
> which is the same as if(opt_opts.headers).
> 
> But still doesn't work in reloading the diff page. For example going to
> http://gerrit-new.wmflabs.org/r/c/2/62/tests/fixtures/layout-cloner.yaml and
> then reloading that downloads a json file (the fix i did for headers is not
> on that site). It should not be downloading json files when refreshing the
> page. Which i presume thats why headers: is there to fix that.

If it is still not working, there might be another underlying bug somewhere.
Can you try checking the headers purpose and see whether there is something different with the patch in network inspector?
Time permitting, I can also look at it if I got access to a web site with this patch on.

[Radar WebKit Bug Importer]

<rdar://problem/31278173>

[paladox]

Looking into it more. I'm thinking it's https://gerrit-review.googlesource.com/#/c/100912/ . As I merged something into that patch and also updated it which may have lead to the problem with refreshing diffs. As leading the headers: as it is works in chrome but refreshing a diff fails too.

Must be the way I updated https://gerrit-review.googlesource.com/#/c/100912/

[youenn fablet]

paradox, can we close the bug here as duplicate of https://bugs.webkit.org/show_bug.cgi?id=168043, or is there some additional issue?

[paladox]

Yep we can close this as duplicate of https://bugs.webkit.org/show_bug.cgi?id=168043

[Alexey Proskuryakov]

*** This bug has been marked as a duplicate of bug 168043 ***