Error handling

[robertocarroll]

Keeping a list of the things that need to be addressed after the prototype:

0. Loading the dictionary

   • [ ] If the dictionary doesn't load, tell the user
   • [ ] Provide a way to try again

1. Loading the API

   • [x] Disable / hide nominate button until Facebook API is connected.
   • [x] Error handling on API connection fails - if fails, tell the user
   • [x] Provide a way to try again

2. Logging in

   • [x] Error handling on Facebook login - if fails, tell the user
   • [x] Provide a way to try again

3. Getting friends

   • [x] Error handling on getting friends - if fails, tell the user
   • [x] Provide a way to try again

4. Post message

   • [x] Validation / Error handling on form - if empty, tell the user - think it's OK to have an empty form
   • [x] Error handling on posting message - if fails, tell the user
   • [x] Provide a way to try again

[pauldwaite]

Possible non-handled errors:

• [x] https://github.com/wearethoughtfox/amnesty-facebook/issues/48 (user has no Facebook friends)
• [x] https://github.com/wearethoughtfox/amnesty-facebook/issues/47 (user logs out of Facebook in another tab/window)

[robertocarroll]

I've done some work on the error handling in this branch:
https://github.com/wearethoughtfox/amnesty-facebook/commits/errorhandling

A couple of the things aren't tested.

• Any ideas how to simulate error messages in the API?
• Can you see any documentation on handling errors with loading the SDK? This is the best I could find: http://jsfiddle.net/PRvJs/1/

Please can you have a look when you get chance and see what you think.

[pauldwaite]

@robertocarroll

The error handling branch looks good to me — it successfully fixes #47 and #48, which were the error-handling-y bugs I spotted.

Any ideas how to simulate error messages in the API?

Not sure. I’ve been testing #47 and #48 by:

1. Logging in as test users (one with friends for 47, and without for 48)
2. For 47, logging out again before trying to post a message.

Can you see any documentation on handling errors with loading the SDK?

The Facebook API documentation befuddles me. But basically, to load the SDK, we add a <script> tag to the page, right? Or rather, this code from Facebook does:

(function(d, s, id){
     var js, fjs = d.getElementsByTagName(s)[0];
     if (d.getElementById(id)) {return;}
     js = d.createElement(s); js.id = id;
     js.src = "//connect.facebook.net/en_US/sdk.js";
     fjs.parentNode.insertBefore(js, fjs);
   }(document, 'script', 'facebook-jssdk'));

If that file loads successfully, it calls window.fbAsyncInit, in which we call FB.init.

So, there’s two things we could do:

1. Add an onerror handler to the Facebook SDK <script> tag, so that we know if it explicitly fails to load.
2. Presumably, it’s possible for FB.init to not work, like if we pass the wrong app id. We could try that and see what happens, and then work out how best to handle it. (Which will probably come down to asking the user to refresh the page, but hey.)

[robertocarroll]

Having an error handler in the init sounds good.

I found a couple of things while roving around:

• https://stackoverflow.com/questions/3548493/how-to-detect-when-facebooks-fb-init-is-complete
• https://github.com/tjmehta/fbExec.js

which might help. Let me know what you think and then we can wrap this one up.

[pauldwaite]

Sure. I don’t think fbExec.js does much for us — if I understand it right, we’d have to replace all our FB calls with fbExec, and we’re solving the problem in a different way: specifically, with promises.

I’ve updated the branch with checks for the Facebook SDK having a load error, and for it taking longer than 60 seconds to load. I’m not doing anything when those happen, but I’ve commented where we would do something.

To simulate the former, changing the url in the following line to something nonsensical should do it:

js.src = "//connect.facebook.net/en_US/sdk.js";

To simulate the latter, I think running the app on your laptop and then turning off your internet access before loading it should do it.

[robertocarroll]

I thought this would be straightforward, but of course the dictionary file hasn't loaded so we don't have any strings.

[pauldwaite]

of course the dictionary file hasn't loaded

Now that dictionary.loadStrings returns a promise, we can wait for the dictionary to load before taking actions that rely on it. For example, we can change our document ready function from this:

$(document).ready(function(){
  Promise.all([
    amnestyApp.loadFacebookApi,
    dictionary.loadStrings("data/dictionary.json")
  ]).then(function () {
    amnestyApp.start();
  }, function (errorDescription) {
    if (errorDescription === "error") {
      //show error message

    }
    else if (errorDescription === "timeout") {
      //show error message
    }
  });
});

To this:

$(document).ready(function(){
  amnestyApp.dictionaryPromise = dictionary.loadStrings("data/dictionary.json");
  Promise.all([
    amnestyApp.loadFacebookApi,
    amnestyApp.dictionaryPromise
  ]).then(function () {
    amnestyApp.start();
  }, function (errorDescription) {
    if (errorDescription === "error") {
      //show error message

    }
    else if (errorDescription === "timeout") {
      //show error message
    }
  });
});

Then either in the document ready function, or anywhere else in the app, to do something after the dictionary has loaded, we can do this:

amnestyApp.dictionaryPromise.then(function () {
    // This code will run once the dictionary has loaded.
});

[robertocarroll]

I was about to give this a whirl, but slight confusion. In your example above, it already contains everything we need doesn't it?

You're calling amnestyApp.dictionaryPromise.then before amnestyApp.start. So if I just switch the current code for that code and add in views for Facebook loading errors that should be it shouldn't it?

[pauldwaite]

In the second example above, we wait for two things to happen before calling amnestyApp.start():

1. the dictionary loading (as monitored by amnestyApp.dictionaryPromise — quite possibly better to call that amnestyApp.loadDictionary or something)
2. the Facebook API loading (as monitored by amnestyApp.loadFacebookApi)

As such, if there are Facebook API load errors, amnestyApp.start() won’t be called.

I suggested keeping a reference to the dictionary loading promise because you said

of course the dictionary file hasn't loaded so we don't have any strings

I figured you meant that in order to show a Facebook API load error, we’d need the dictionary file to get our Facebook API-related error strings.

[robertocarroll]

I must admit I'm a bit stumped with the Facebook initial error. I was thinking I could create a model that says Facebook error is true or false and then go on the start the app and depending on the value of the model either start the app as normal or show an error.

But is that too complicated? I couldn't get the text to show because all the app dictionary stuff hadn't fired up at the point of the Facebook API errors.

It would be great to hear if you have a simple answer.

[pauldwaite]

Sure. I’m not sure it needs a model, but I’m not very familiar with Marionette conventions.

all the app dictionary stuff hadn't fired up at the point of the Facebook API errors

I don’t think that’s necessarily true. The dictionary and the Facebook API are independent; if the Facebook API load times out, for example, the dictionary will most likely have loaded.

Either way, I think the solution is to use the dictionary promise to make sure we only run the code that shows the Facebook error once the dictionary has loaded. I’ve pushed a commit to master that does this (https://github.com/wearethoughtfox/amnesty-facebook/commit/7a54dbabec91bcdd4e1b52467cda54046f46e911); it seems to work.

(I tested using the two methods I described above — loading the app having turned my laptop’s wifi off, and loading the app having changed the Facebook SDK URL to something nonsensical. In both cases the Facebook error message was displayed.)

[robertocarroll]

Excellent. Thanks!