Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors not getting thrown in Cordova #66

Open
antonio0 opened this issue Sep 9, 2013 · 7 comments
Open

Errors not getting thrown in Cordova #66

antonio0 opened this issue Sep 9, 2013 · 7 comments

Comments

@antonio0
Copy link

antonio0 commented Sep 9, 2013

Hi everyone,

I'm using the following structure in order to do something depending on whether a template file is found or not:

require ( [text!filename],
function(file) { console.log('success') },
function(err) { console.log('error') } );

This works fine on chrome but, when on a Cordova/Phonegap app, it always returns 'success', even if the file does not exist.

@jrburke
Copy link
Member

jrburke commented Sep 20, 2013

It would be helpful to know what Cordova/Phonegap combo was used, and on what platform. If you can find out what the HTTP status code is for the value returned here:

https://github.com/requirejs/text/blob/master/text.js#L288

that would help too. Also log out the URL, maybe there is something funky with the type of protocol/scheme that is uesd (like 'http:' or 'app:').

@cdmahoney
Copy link

I am having a similar problem - test code worked in chrome, fails on Android. The problem (in my case) is that XMLHttpRequest returns status code 0 when the file is not found.

The status code check in line 289 (v2.0.10) doesn't take into account this possibility:
if (status > 399 && status < 600) {
...

Modifying the condition to:
if (status === 0 || (status > 399 && status < 600)) {

makes things work as expected.

@jrburke jrburke added this to the 2.0.11 milestone Apr 14, 2014
@jrburke
Copy link
Member

jrburke commented Apr 14, 2014

Fixed and released 2.0.11 with the fix.

@sjamaan
Copy link

sjamaan commented Apr 14, 2014

I just noticed that our code started breaking after auto-updating to this minor version :(

It appears that this change breaks code in PhantomJS. I suspect that more generally it breaks when using the file:/// protocol: since there is no HTTP request, there is no sensible status code it can use. For the time being I've downgraded to 2.0.10, but this should really be fixed.

@jrburke jrburke modified the milestones: 2.0.12, 2.0.11 Apr 14, 2014
@jrburke
Copy link
Member

jrburke commented Apr 14, 2014

Wow, I messed that up. I confused allowing for zero for local files and the issue reported here, which is android specific for a failed file. I reverted the change and released 2.0.12.

So for Android where it reports 0 for a failed XHR call, I will likely need a test case/Android version failure range, or some other check than the status code. Unfortunately I am not set up to do this sort of test easily. Will reopen to allow others to discuss and find a fix.

@jrburke jrburke reopened this Apr 14, 2014
@sjamaan
Copy link

sjamaan commented Apr 14, 2014

Cool, thanks for the quick action! I've been using requirejs on a Cordova project without trouble, but I haven't tested what it does with failed calls. I can't promise anything, but when I have time I could look into it, if others haven't in the meanwhile.

@csimi
Copy link

csimi commented Jan 21, 2016

This is also an issue with cross-domain requests.
The onreadystatechange handler gets called with a status of 0 in case of an error if the server is set up to only return a CORS header on success.
Looks like NGINX only returns added headers for specific 2xx and 3xx responses by default, unless you use the always parameter. A bunch of CORS tutorials don't mention this.
Using the onload and onerror callbacks in browsers that support them would ensure that unexpected errors such as these call the errback.
It's also worth noting that if you want to support IE9-10 CORS requests by returning an XDomainRequest in config.createXhr, it's completely missing onreadystatechange support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants