481319 - Weave.Service needs a way for UI code to query current status or last error

Status: RESOLVED FIXED

(Cloud Services :: General, defect, P1)

Description

[Jono Xia]

When UI code calls Weave.Service.login() or Weave.Service.sync(), those are internally (in service.js) wrapped in Wrap._catchAll, so any exceptions that they raise get caught and logged.  The problem is that the UI code doesn't get anything back except a True or a False -- if it's false, there's no way to see what the exception was, so there's no way for the UI to display to the user what the *reason* was for the failure.

I propose keeping some kind of ._currentStatus or ._lastError field around in the service, so that UI code can query it after a failure occurs, to tell the user what went wrong.  It could be a string or a constant error code defined somewhere.

Comment 1

[Jono Xia]

Partially done in http://hg.mozilla.org/labs/weave/rev/fcac59ed1f68 which adds Weave.Service.mostRecentError (a string).

Comment 2

[Jono Xia]

Make it store an object instead of a string, so it can keep extra data like server response code etc.

Comment 3

[Dan Mills [:thunder]]

To clarify, I think we need:

1) a getter that returns a short status summary, I think there are at least 3 states:

- hard failure (e.g., can't log in, server offline, etc)
- partial failure (e.g., some engines can't sync, but others can)
- success (no problems whatsoever at this time)

2) a getter that returns more detailed information about the above status:

- exceptions, if any
- which engines are failing, if any
- possible causes of failures, if known

These two getters could potentially be the same one, so long as it is easy (one line) to check the summary.

We need to define the data structure for the details.

Comment 4

[Jono Xia]

Largely done in http://hg.mozilla.org/labs/weave/rev/c4310793269e

The getters are:
Weave.Service.statusCode   (one of WEAVE_STATUS_OK, WEAVE_STATUS_FAIL, or WEAVE_STATUS_PARTIAL)

Weave.Service.detailedStatus   (returns a data structure like the following:)

{ server: [ statusCode, anotherStatusCode ],
  sync: statusCode,
  engines: {  passwords: exception,
              tabs: exception }
}

Constants are defined in modules/constants.js

Currently for engines, if nothing went wrong with an engine then there is just no entry for that engine in engines.  If an exception occured, then the key is engine.name and the value is the exception.  Should we instead put in a status code on engine success?

The server status array is currently not used; we need to implement detection of http headers from the server.

We could start using the new status codes and object in a lot of places: Currently only Fennec UI uses the detailed status object; firefox UI does not.  Currently nobody is using Weave.Service.statusCode.  It has a lot of overlap with existing mechanisms (basically redundant with many of the exceptions that are thrown from Weave.Service, with the return values of Weave.Service.sync and Weave.Service.status, etc.)  

Also, we should make sure to use localization whenever displaying status strings, instead of displaying the bare string values of the status code constants.

Comment 5

[Ed Lee :Mardak]

(In reply to comment #4)
> Constants are defined in modules/constants.js
There weren't any changes for that file in the revision

Comment 6

[Dan Mills [:thunder]]

Oh, yes.

Jono, note that you have to make the changes in modules/constants.js.in

Comment 7

[Jono Xia]

Oh.  Duh.  Fixed in http://hg.mozilla.org/labs/weave/rev/6770368ee44a