garden-o-matic broken: TypeError: 'undefined' is not an object (evaluating 'buildLocations[currentIndex].url')

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Loading http://build.webkit.org/TestFailures/garden-o-matic.html?platform=apple, I get:
TypeError: 'undefined' is not an object (evaluating 'buildLocations[currentIndex].url')
and it never gets past Loading commit data ...

[Simon Fraser (smfr)]

I'm seeing parse errors from net.jsonp for full_results.json files. If I try to validate them, validation fails with the ADD_RESULTS() stuff. How did this ever work?

[Simon Fraser (smfr)]

Also, the mac testers can now do testing out of order, so we can't just go by build number, and assume that the most recent build is the most recent revision.

[Adam Barth]

net.jsonp strips out the ADD_RESULTS part of the response.

[Ojan Vafai]

(In reply to comment #1)
> I'm seeing parse errors from net.jsonp for full_results.json files. If I try to validate them, validation fails with the ADD_RESULTS() stuff. How did this ever work?

Are you sure you diagnosed this right? I'm not seeing parse errors. I get an error on line 629 of results.js:
net.jsonp(buildLocations[currentIndex].url, resultsCallback);
Uncaught TypeError: Cannot read property 'url' of undefined 

So, somehow currentIndex is exceeding the bounds of buildLocations. This is the call that fetches full_results.json.

[Ojan Vafai]

Also, I'm looking at http://trac.webkit.org/export/HEAD/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/garden-o-matic.html?platform=apple since http://build.webkit.org/TestFailures isn't always up to date. I'm not really sure how/when that gets pushed.

[Ojan Vafai]

Ugh. Sorry, I spoke before understanding the code in question. Ignore me. :(

[Ojan Vafai]

That all said, I can't figure out what's going wrong here. It looks like we never hit the success clause in net.jsonp, but I can see in the inspector the some full_results.json requests are returning 200s with valid data.

[Simon Fraser (smfr)]

(In reply to comment #7)
> That all said, I can't figure out what's going wrong here. It looks like we never hit the success clause in net.jsonp, but I can see in the inspector the some full_results.json requests are returning 200s with valid data.

I understand why we can walk off the end of buildLocations.

But I was seeing net.jsonp report parseErrors when fetching full_results.jsonp, so it was able to download results from the server, but all the results files failed to parse. That's why it hit the end of the list of buildLocations.

[Simon Fraser (smfr)]

Oddly I don't see the same issue on my work machine (or something was fixed overnight).

[Ojan Vafai]

I'm pretty sure jQuery's ajax library is doing something crazy here. If I break in the error handler of $.ajax, I get a "parsererror" error. But if I look at the responseText, it's perfectly valid JSONP as best I can tell. jquery shouldn't even know that this is supposed to be json though. Maybe it's trying to parse the content as json for some reason?

[Ojan Vafai]

Found the problem. Not sure how this ever worked.

http://api.jquery.com/jQuery.ajax/

dataType
Default: Intelligent Guess (xml, json, script, or html)
The type of data that you're expecting back from the server. If none is specified, jQuery will try to infer it based on the MIME type of the response..."json": Evaluates the response as JSON and returns a JavaScript object. In jQuery 1.4 the JSON data is parsed in a strict manner; any malformed JSON is rejected and a parse error is thrown. (See json.org for more information on proper JSON formatting.)

I think setting the dataType to jsonp might fix this. Or, we could just use XHR directly. We don't actually need any of jquery's magic here. We're just using it for onload/onerror anyways.

[Adam Barth]

+1 for using XHR directly.  We started out using jQuery but now regret it.

[Ojan Vafai]

Created attachment 150274 [details]
Patch

[Ojan Vafai]

FWIW, I would guess that a server-side change to the content-type of the json files is what broke this.

[WebKit Review Bot]

Comment on attachment 150274 [details]
Patch

Clearing flags on attachment: 150274

Committed r121617: <http://trac.webkit.org/changeset/121617>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.