sysapps / telephony

API to manage telephony calls
15 stars 12 forks source link

Evaluate spec for use of Promises #17

Open marcoscaceres opened 11 years ago

marcoscaceres commented 11 years ago

The spec should be evaluated for potential uses of Promises. There are a number of places where Events are being used to deliver Errors async, where a future would be more appropriate.

zolkis commented 11 years ago

What is the implementation status of Future?

marcoscaceres commented 11 years ago

There is work happening in both WebKit (by @cdumez, I believe) and is planned for Firefox (https://bugzilla.mozilla.org/show_bug.cgi?id=856410). I'm not sure what is happening in Chrome, but Alex Russell from the Chrome team has been the lead driver of Futures (so I assume it will be in Blink). In any case, there is strong support for futures from the browser vendors.

zolkis commented 11 years ago

Yes, Chris mentioned there have been some issues (moving spec, etc). I think I will just use a TelephonyRequest pending object, which can use Future in the future, for now I'll just "simulate" it in the implementation.

marcoscaceres commented 11 years ago

I thought we had consensus to move all the APIs to futures (need to find link from @mounirlamouri in mail archive). Having TelephonyRequest simply won't fly as it will be rejected out of hand if this document goes to Last Call. It's best to remove TelephonyRequest early and fix any issues we find with Futures and its relationship to this API.

zolkis commented 11 years ago

Yes, the consensus was that we will move the API's to use Future's. But if we move the spec to use Future now, it must be implementable/usable now. I am implementing the Telephony API now. Does it mean I have to implement Future as well? Yes - but I'd like to avoid that.

marcoscaceres commented 11 years ago

The problem is that if you implement the API and developers start using it, we are going to have problems. This is why we need to push more quickly towards making futures more stable. Also, I would caution doing a serious implementation of the API at this point - though a prototype implementation would be great. The API has barely been reviewed yet and could still go through fairly significant changes.

zolkis commented 11 years ago

Nono, it's just a prototype implementation in JS, plus a simple dialer app, just to prove the API design, and have some fun :). So it's not that critical whether I use Future, or a simulation of it, at this point.

But if the spec moves forward and implementations have to use Future, it has to be available. Of course, big changes could still occur (for one, I too have made a proposal - which I am going to try out as well by implementing a proto for it).

marcoscaceres commented 11 years ago

Ah! man! that's great to hear! You can just grab the Futures polyfill (it matches the current spec): https://github.com/slightlyoff/DOMFuture

Also, happy to help with the prototyping. I have a WebIDL library that can be used to do a lot of the conversions. https://github.com/extensibleweb/webidl.js

Let me know if you want me to help there. I'm happy to plug in the WebIDL stuff if you do the algorithms. Can also help with testing.

marcoscaceres commented 11 years ago

do the algorithms === code the algorithms in the spec in JS.

zolkis commented 11 years ago

Sounds great, thanks for the heads up. I will look into these.

marcoscaceres commented 11 years ago

So, my TelephonyManager evaluation is that the following are good fits for futures (as they are asyc in their interaction with the telephony backend, and can result in an error):

From TelephonyManager:

TelephonyCall:

We might be able to get rid of some events as a result. Depends if this object is expected to have access to call it did not manage.

zolkis commented 11 years ago

My list would be: TelephonyManager. sendTones() startTone() endTone()

The rest is asynchronous operation controlled by TelephonyCall, which can be considered a special Future. It manages its own state and errors. If there is an error, there should be an event about it.

hsinyi commented 11 years ago

In addition to Zoltan's list, dial() and join() should be probably added IMHO, as they are asyn and it makes sense that a new call object is created and returned when the request succeeds.

I agree with Zoltan on the rest operations controlled by TelephonyCall.

Best regards, Hsinyi

On 05/25/2013 05:18 AM, Zoltan Kis wrote:

My list would be: TelephonyManager. sendTones() startTone() endTone()

The rest is asynchronous operation controlled by TelephonyCall, which can be considered a special Future. It manages its own state and errors. If there is an error, there should be an event about it.

— Reply to this email directly or view it on GitHub https://github.com/sysapps/telephony/issues/17#issuecomment-18430548.

zolkis commented 11 years ago

dial() should return a TelephonyCall, just as it would return a Future. If anything goes wrong with the call (no telephony service etc), it is signalled through an error event, or through a call state change and disconnect reason.

Similarly, join() returns a ConferenceCall, just as it would return a Future - the same way as dial().

The telephony states and use cases are much more complex than what Futures support, that is why a special async handler is needed. Using TelephonyCall is also more intuitive and better known to dialer developers, than breaking out of the box and rethink call state transitions as arbitrary chaining of then() calls on Futures.

So my suggestion is to go on with this as it is, except the tone handling, where the Futures can be used. Similarly, Futures can be used on telephony services supported in the future, which don't have their own state management.

zolkis commented 11 years ago

Linked to FPWD, for sendTones(), startTone(), endTone(), to update WebIDL + steps. Work started.

zolkis commented 11 years ago

PR #83 closes this. For new opportunities, reopen and update the milestone.

marcoscaceres commented 11 years ago

Hold, resume, and disconnect still feel like good candidates for returning a promise.

zolkis commented 11 years ago

I don't get why. TelephonyCall itself is the 'promise'.

domenic commented 11 years ago

I don't get why. TelephonyCall itself is the 'promise'.

From what I can tell, TelephonyCall seems to represent a stateful system, not a asynchronous return value from a single asynchronous operation.

zolkis commented 11 years ago

@domenic I agree.

marcoscaceres commented 11 years ago

I also agree, but we are conflating two things: the creation of an object instance and the asynchronous operation dial() - which does meet the definition of a promise. IMO, it would be better if TelephonyCall had a constructor and the one could dial() it:

var tel = new TelephonyCall();
tel.dial(num).then(...);

//or even (create but don't dial) as each call obj should only be used once
var tel = new TelephonyCall(num);
// time passes, othe things happen.
tel.dial().then(...);

It sounds a bit like what Tab is talking about here: http://lists.w3.org/Archives/Public/public-script-coord/2013AprJun/0738.html

When we're trying to construct an object, the right idiom is to use a constructor, rather than a factory function. On the other hand, when we need to return something async, the right idiom is to use a promise.

A TelephonyCall does not need to be asynchronously created before it is dialed. Dialing must be done async, however.

Or am I not getting it?

zolkis commented 11 years ago

A TelephonyCall object has no right to exist, unless it is already dialing or in any next state of a call, or if incoming/waiting. The dial() method or the 'incoming' event creates the object which can handle the lifecycle of the call. It does not make any sense to 'create a call', that would be rather named 'dial a call'. Dial is not a simple asynchronous operation which ends and returns a result, but rather, as Domenic noticed, it starts a state machine with a lots of possible ramifications, also depending on user input. Therefore the 'promise' pattern does not apply IMO.

tabatkins commented 11 years ago

Given the current state of the .dial() function, where it syncly creates a TelephonyCall and then starts dialing async, there's no reason not to just kill it in favor of a TelephoneCall constructor with an equivalent signature. Using a factory function is just bad idiom.

If you changed the return value of .dial() to a Promise for a dialing TelephonyCall, you could eliminate the "dialing" event, but you'd have to switch back to a factory function - the consensus seems to be to add it to the interface it's creating, as a static function, idiomatically named "create" (though I suspect there's room for more informative names in cases that can justify it).

(The same applies to createConference - it's currently a non-idiomatic factory function. One could make a constructor for it that takes either a TelephonyCall or a number to dial.)

Looking more into the states, I agree that some, possibly even all, of the state events can be dropped for Promises (though there may still be reasons to keep some of them).

In general, state machines are a poor match for Promises, since you can often return to the same state more than once and state changes can occur without script intervention. This means it's hard to tell, when script requests a promise for a given state, whether it's asking for the last time that state was entered, or the next time. Further, state changes may occur faster than the script can request promises, so you miss things.

In this specific case, though, these problems aren't present. Some states can be entered multiple times, but they're always caused by explicit script action, calling a method on the Call object. Those methods can return Promises for the next state. This avoids the prev/next ambiguity, and removes any possibility of missing too-fast transitions. If the transition fails due to a disconnection, the promise can reject.

(It looks like the state machine graph isn't complete, though - there are several more events in the IDL that don't have corresponding boxes. I'd be interested to see those in the graph, so I don't have to go through the technical details of the algorithms to understand the transitions.)

The one exception to the "every state change is explicit" is transitions to "disconnected", which can happen at any time. Luckily, this only happens once per Call, so we can just attach a .disconnected promise to the call object, which accepts when the Call is disconnected.

This allows us to wipe out all the events, but it could be useful to add some back. The question is, is there a reason for other scripts (not communicating with the calling script) to observe and react to the state changes? If so, the states they have reason to observe need to be events as well.

(Note that it's fine to have both an event and a promise for the same thing, as they serve different purposes. For example, the Font Load Events API, once it's changed to reflect the resolutions from our last F2F, will have a .ready() method returning a promise which accepts if/when there are no pending font loads, and a pair of loading/loaded events which fire when there are/aren't pending font loads. The former is useful for script that doesn't want to track font load state normally, but occasionally wants to make sure some measurements they'll make, which are dependent on font metrics, will be accurate; the latter is useful for anything that actually wants to track font load state explicitly across the document.

zolkis commented 11 years ago

Promises handle a single asynchronous request, with success and a return value, or error. TelephonyCall can enter multiple possible states from a given state, even error conditions, and come back to a valid state. If you introduce Promises that can handle multiple possible outcomes, we may use them. Until then implementations will struggle a lot to make telephony calls work with promises, especially with corner cases. I encourage you to design and implement this API, together with a real dialer, using Promises for state handling, and make it work in a real telephony network. I'd like to see your JS code after you have fixed all bugs... Remember that state transitions may be dependent on protocol, network, even radio conditions, and the job of any app using this API to track the network states as closely as possible, and not to introduce an intermediary internal state machine with all complications, just in order to force describing the world with the latest favorite Promises.

About constructors. As said, a TelephonyCall has no right to exist before a call is being dialed, or received. So constructors are not a good match in this case. If for some philosophical, esthetic or religious reason we want to use constructors and only constructors, we could introduce a new state 'idle', meaning there is no call and the object is invalid (i.e. no counterpart in any telephony service/middleware). The API just becomes more verbose and less consistent with itself and telephony services, but that may be a small price to pay for a perfect world.

For the continuation of the discussion, I suggest proving the alternative design using constructors/Promises with a consistent WebIDL and example code in a separate branch.

domenic commented 11 years ago

@zolkis I think you may be talking past us. Our thesis is that methods that trigger asynchronous state transitions should return promises. These are indeed single asynchronous operations which can either succeed or error, and thus a perfect place to use promises.

I think @marcoscaceres's point when he said

we are conflating two things: the creation of an object instance and the asynchronous operation dial()

was that one of these asynchronous state transitions is indeed the transition from "initial state", call it "idle," to the dialed state. Whether that makes sense in this domain is outside my expertise, but I do think it's bizarre to be specifying a constructor function, complete with prototype properties etc., without actually allowing you to call that constructor.

zolkis commented 11 years ago

Let's suppose you are implementing a constructor. From where do you fill the needed properties, unless you have a real object in telephony middleware/ network which provides you the data needed for the telephony call? The implementation does not create a call. The telephony network does. So it definitely does not make any sense to create a TelephonyCall object by any developer initiative, since it will have no data. Of course no one would prevent developers from filling the memory with useless TelephonyCall objects, but there must be a mechanism to tell the valid ones from the invalid ones.

Then, about the promises, let's talk by examples. Suppose if dial() was to return a Promise, in success case returning a TelephonyCall. Even in the error case, there should be a TelephonyCall object, and any incident is to be logged in call history. So there is no error branch at the Promise level, as it should be handled inside TelephonyCall.

For other methods, e.g. hold(), we could replace the 'holding' state with a Promise, but it is unclear what is the sequence of operations on success, since the then() is called, but the 'statechanged' and 'held' events are also fired at TelephonyCall. It would be awkward for telephony people to deal with promises instead of dealing with well known call states. It would be really hard to track a call, and especially hard to tell in which state a given call is e.g. when the dialer is recovering from a crash. As I said, this discussion only makes sense if someone consistently designs the whole API with your preferred patterns, and validate that design.

tabatkins commented 11 years ago

@zolkis You definitely didn't read my post in detail, as your objections don't make any sense.

For example, my very first paragraph explained how using a constructor was just moving a function around, not changing any details. "TelephonyManager.dial(num)" and "new TelephonyCall(num)" are identical, except that the latter is more idiomatic.

I then explained how you could get rid of the dialing state event by having the dialing action return a promise for a dialed call. That prevents you from using a constructor, but the correct idiom there is a factory function on the interface itself, not on a manager interface: "TelephonyCall.create(num)" or "TelephonyCall.dial(num)".


Regarding the state machine, as Domenic said, as far as the spec's state machine diagram shows, every state transition is triggered by an explicit method call, which can have only two outcomes - the call switches to the desired state, or it disconnects. This matches the Promises model well. (The one exception is that disconnection can happen at any time, but it's also a one-time thing, which is well-handled by a promise.) Are there exceptions? I know the state machine diagram is incomplete, as I said, because there are more states in the IDL than show up in the diagram. Are some of the IDL states possible to enter without a script calling a method? If so, the state machine diagram should be augmented.

I'm not certain that a promise-based design is best for this, but it seems like it is possible. You've yet to give actual details as to why it would work badly, unfortunately. If you'd like to give real examples, I can walk through them and explain whether or not they would work in a promise-based design.

Otherwise, I gave reasons why it might still make sense to maintain some of the events. You didn't even address that part in your reply.

zolkis commented 11 years ago

@tabatkins I've read your comments and I claim I even understood it, but it is in a pure 'theory of JS' context. I speak in telephony context (that's why we talk past each other, as @domenic has put it). In telephony contex, using these ideas messes up quite a lot of things, since you are making assumptions that do not hold, with the main purpose just to push using Promises in the API. I say that Telephony is not the best application field for Promises (as they are specified today), unlike Messaging for instance. I think using Promises should have less priority than having a correct Telephony API. Some comments.

  1. Subjectively, I'd rather use the name 'dial()' than 'new TelephonyCall(number)'.
  2. BTW, specifying the number is not enough, as a call is made on a telephony service. There may be many telephony services available. Technically the 'dial()' method belongs to a TelephonyService (or manager) object, rather than the TeleponyCall object, since the latter has no right to exist before dial() is called, since that is supposed to create the TelephonyCall object (I speak in telephony domain context, not in generic programming context).
  3. We could modify the dial() method to return a Promise which in both success and error cases returns a TelephonyCall object. We need the object even in error case, since there are errors which may be signalled separately by the telephony middleware (e.g. failure/disconnect reason).
  4. Then, the state machine. This is an evergreen discussion, since people don't get that the telephony state machine is not guaranteed (by the network). It gives a desirable succession of events, but it is owned by the telephony network. This is the reason I opposed depicting a state machine in the spec. Implementations should be transparent with states and report them ASAP to apps. 'disconnect' is not the only state that may happen as an alternative to any state. As the device travels through various telephony networks, some states may be skipped; important is that the app is notified faithfully about the current state, and if it has any internal states, should resolve them accordingly. If instead of using a state machine you create a programming model with possibly multiple active Promises (analogous to multiple message reception points), then managing the state machine becomes a nightmare, and will certainly fail. Even if this is solved by not allowing this, there is the basic problem with the assumption that it works like you dial a call, and it either succeeds or fails. Or, for any other state. It can succeed with multiple possible new states, and it can fail in multiple ways. You dial a call, and it may enter in voice mail mode from the network, or get forwarded, or disconnected with an error, or connected with voice.
  5. About createConference there was a complete misunderstanding, as it is not a factory method at all. In CS telephony it means a 'join' operation, merging all calls of the subscriber (in the telephony network side), and provide a structure to control the multiparty call. This should happen in one asynchronous operation, indeed. That is what is happening in telephony. So it has to be a method without any parameters, and create a control object, both in success and error cases. I would like to see your proposal with Promises for this, and other use cases.

The current API with the state machine model does work with telephony, with the remark that we should explore using TelephonyService objects (slowly we are converging towards my original first API from one year ago...). From my POV this discussion is blowing up the work of the past year. May be a good thing. In any case, IMO we need to generalize Promises to handle multiple possible next states (instead of just success and error cases), in order to apply them in this case. A pure success/error model breaks up operations that ought to be handled in one go.

While it is possible that we may work out a different interaction model for calls using Promises, it would be very different from the current API, and it is best done separately. So what I would like to see is that people who know how they want to implement and use Promises, would make a consistent proposal on how to use these Promises in the Telephony API. I can volunteer to check it from purely telephony context, and tell if it messes up some use cases. Start a branch and work it out. I do not want to enter into theoretical JS discussions in this case; just show what you want, more importantly think it through till the end with all the use cases, and I will tell whether is it correct in telephony context.

zolkis commented 11 years ago

Playing with the idea, here is what we could do with the current API.

  1. dialing a call
// On TelephonyService or TelephonyManager
Promise dial(DOMString remoteParty, optional DialParams params);
//Promise returns a TelephonyCall object both on success, and on error
// From here on, the call is managed by the TelephonyCall object

Steps:

Error path:

Success path:

As one can see, in this particular example the Promise part is basically not needed, as it does not add any information beyond the events. Could you please correct/modify this in the way you imagine Promise should work?

Additional items to explore:

// on TelephonyCall
Promise createConference ();
// Promise returns a ConferenceCall object on success, and an error code on error, 
// leaving the 'this' TelephonyCall unchanged.
//
// On ConferenceCall
Promise split (TelephonyCall participantCall);
// Promise can simply succeed (split call being activated, no special handling needed) 
// or err (conference call remains intact, error could be logged).
tabatkins commented 11 years ago

Okay, cool, that's all useful information. It seems like, in general, the state machine is not amenable to promises. No problem! This wasn't clear from the spec, but your additional info makes it pretty clear.

(I don't think this is clear in the spec at all, even ignoring the state diagrams. The algorithms seem incomplete, and don't even hint that the state could change to something other than what's expected. I don't think I could even come close to implementing the spec correctly in its current state, which also makes it difficult to comment about the design of the spec, as we've discovered.)


Regarding the constructor/dial() method, everyone always thinks they have a good reason to break from the idiom. ^_^ This use-case isn't special. You're creating a new object, and so it should use a constructor unless the object must be returned asynchronously.

In particular, arguments about what it belongs to don't seem to matter here. The TelephonyManager is a per-document singleton, so merely from the knowledge of what document the script is in, you can associate a new TelephonyCall with its TelephonyManager automatically; you don't need to explicitly call a method on TelephonyManager to make the association. If we keep with the current design, where even the dialing state is a node in the state machine, then "new TelephonyCall(num)" is perfectly appropriate, and more idiomatic.

While we could go with a Promise for the result of dialing, given that we can't promise-ify nearly any of the rest of the state machine, I don't see a particularly good reason to do it for this either.


That said, there may still be a good reason to use a "disconnected" promise. It seems to fit the definition of a promise perfectly - it's a one-time event, and it's useful to both wait for it before it occurs, and to respond to it after it occurs. This would just be a Promise hanging off of the TelephonyCall interface, which accepts with all the necessary information when the call disconnects for any reason.

This adds useful information, because it makes it impossible for someone to "miss" the disconnection by being handed the TelephoneCall object after the disconnect event has already been fired.


Regarding createConference(), I called it a factory method because it's a constructor, but exists in the form of a method on some other interface. Again, constructors are idiomatic and to be preferred unless there's a good reason to do otherwise.

In particular, looking into the details of createConference(), it just merges all the current live calls. You don't even need any additional arguments! Just "new ConferenceCall()", and that's that. It doesn't need any special information from the current active call (and anyway, the identity of the current active call can be passively obtained), and the held calls are already passively obtained.


ConferenceCall#split() does seem reasonable to upgrade into a Promise, but given that all it does in success is put the conf call on hold and the tel call on active, and the script probably already has event handlers for those set up, I dunno how useful it is.

zolkis commented 11 years ago

One point you've missed from the above is that we also need to specify the telephony service to be used, and that telephony service may grow to be visible objects in next versions of the API. In that case we'd like to create calls on the TelephonyService objects, which would contain a list of TelephonyCall objects that they manage.

The TelephonyManager would mainly manage the TelephonyServices.

With this in mind, it would be awkward saying 'new TelephonyCall(remoteParty, dialOptions)'; It would feel more natural saying 'telephonyService.dial(number, dialOptions)', even more because the calls are managed per service.

Also, the 'createConference()' method would be on the TelephonyService object.

In this case, how would you use constructors?

tabatkins commented 11 years ago

I can't be blamed for missing a detail that's not in the spec. ^_^ Right now, the telephony service is just a string. Upgrading it to a real object requires no changes; the IDL will just be changed to a union.

(If you're planning to do this, I recommend changing the DialParams key to just "service", rather than the current "serviceId".)

Even if you feel that service.dial(num, options) is significantly better than new TelephonyCall(num, options), it's still worthwhile to provide the idiomatic pattern. Convenience methods are fine to add on top.

zolkis commented 11 years ago

I can't be blamed for missing a detail that's not in the spec. ^_^

Of course, I did not mean to blame you. It is not in the spec, it was in the comments, and likely was not clear.

Even if you feel that service.dial(num, options) is significantly better than new TelephonyCall(num, options), it's still worthwhile to provide the idiomatic pattern. Convenience methods are fine to add on top.

The result of telCall = new TelephonyCall(012345678, { serviceId: "sim1"}) would be that a new element is added to the calls list of the TelephonyService object whose id is "sim1". Not more intuitive to me than telCall = service.dial(012345678); // where service.id === "sim1" . I try to understand why the constructor is better - what are the reasons? What convenience methods did you have in mind?

tabatkins commented 11 years ago

A constructor is useful because that's the idiomatic way to construct an object. You can't predict every way your API will be extended in the future, and providing only convenience methods for constructing the object may bite you in the future when you want to use the object in other ways. You say that a TelephonyCall has no reason to exist unless it's in the act of making a call, but that might not be true forever. Having a constructor protects you at least somewhat from this. (The fact that this is a strong idiom protects this from being just arbitrary over-engineering for undefined future extension.)

It's also nice to have the construction method there in the interface, rather than having to hunt through other interfaces to figure out how to actually create an object of that type.

zolkis commented 11 years ago

OK, I get the point, but it does not convey the right semantics for calls vs services in telephony context.

You can't predict every way your API will be extended in the future

The telephony context is well known, it is driven by rigid standards, and we do know where the API is going in the next few years (CS - VoIP convergence). So there not much danger that the relationship between telephony services and calls would change in the future. However, if we can find a way to use constructors which make sense also in telephony context, I am all ears.

If creating an object that has dependencies on other objects, do you expect to pass everything as parameter to the constructor? (including the service the call is being created on?)

Then, the lifecycle of the call is bound to the service. Service gone, call gone. I guess it is not a problem that an object created by the app is disposed by the implementation.

Also, createConference() could be a constructor for cellular telephony, but it has to be a method of a call in VoIP (or, just skip it and add an addParticipant method to the call object).

Now new TelephonyCall(number, dialOptions) is not the only way to create a call object. TelephonyCall can also be created by the implementation when an 'incoming' event occurs and comes as a property of the event. I assume that is OK.

zolkis commented 11 years ago

A side question about generalizing Promises. @domenic , @tabatkins, @slightlyoff, I am no JS expert, so excuse my ignorance, but based on the discussion above, do you see valid use case to extend Promises to handle multiple outcomes (under a different name, in the example below I use StateHandler for lack of imagination). I believe it would be a more efficient way to handle state handling / events / asynchronous operations in system applications, and could encapsulate the intermediary objects lifecycle such as TelephonyCall. For instance, instead of wiring up event handlers, one could do:

// sim1 is a TelephonyService object
// signature of 'dial()' on TelephonyService is: 
// `StateHandler dial(number, dialOptions);`

stateHandler = sim1.dial("0123456789").on("active", onactive)
                             .on("disconnected", ondisconnected)
                             .on("hold", onhold)
                             .finally(writeCallHistory);

which implies that

telCall = stateHandler.context;
// ...
telCall.hold(); // async op handled by stateHandler
// ...
telCall.disconnect();

Maybe this is not the best way to express it, and may be flawed to start with, but I hope it illustrates what I intended to do.

domenic commented 11 years ago

A side question about generalizing Promises.

Promises represent "asynchronous function calls." They must do one of two things: reject with a single reason, or fulfill with a single value. This is just like synchronous function calls, which can result in a single exception or a single value. So, no, extending them in the way you talk about doesn't really make sense, at least in that those things would not be called promises any more.


The flip side of this is that every asynchronous function should ideally return a promise. The promise may fulfill with undefined, which is fine; many sync operations return undefined, after all. And the promise may never reject, which is fine; many sync operations never throw, after all. But if it's an async operation, you should return a promise, to give a uniform interface for async operations throughout the system.

You can additionally layer a state machine into this, and possibly a bunch of events. But if in any way the operations you're doing are asynchronous, those operations should return promises, regardless of how the rest of the system is structured. Even if those promises always fulfill and their fulfillment value is always undefined, leaving information about what state was transitioned to retrievable through other means, the fact that the state transition took time should be signified by returning a promise.

Hope this helps make things clear. I bow to your superior domain knowledge, which I don't think I'll be able to understand 'cuz it's pretty darn complicated, but in my skewed opinion promise usage is pretty simple, so I hope to hammer home how they work so at least one of us can understand both sides of this conversation :).

zolkis commented 11 years ago

extending them in the way you talk about doesn't really make sense, at least in that those things would not be called promises any more

Yes, I also think they would be a completely different pattern with a different name.

You can additionally layer a state machine into this

So basically you say I should (could) implement state machines with Promises, rather than something like in my example? BTW how does that design with StateHandler feel, is it ok, or flawed, can it be expressed better, is it something broadly useful or something that is ok to be implemented on need?

marcoscaceres commented 11 years ago

I think it might be possible to layer the StateHandler model on top as part of a JS library.

For idiomatic consistency, I would be in favor of changing the spec to use a constructor for TelephonyCall that requires a number, but is optionally initialized with initOptions (a telephony service):

[Constructor(DOMString remoteParty, TelCallOptions initOptions)]
//interface TelephonyCall{ ... }; 

dictionary TelCallOptions{
   TelService service;
}

typedef (DOMstring or TelephonyService) TelService;

With regards to promises, we can continue to evaluate each method on a case by case basis based on @domenic's definition.

@zolkis, let me know if you agree. Happy to keep discussing alternatives.

zolkis commented 11 years ago

I still struggle with this since "making a call" is more than an idiomatic creation of a call object, as it involves telephony network activity. Since you want a synchronous method, you have to separate the creation of the JS object from that of the real call object. As mentioned, such an orphan JS call object has no right to exist. What would the getCalls() method list? This is too much price for an idiom.

Let's clear the design with regards to telephony services first, then return to this topic. I am also interested to see how the other specs are doing this.

marcoscaceres commented 11 years ago

Yeah, I was going to mention potentially looking at similar specs that deal with peer-to-peer communication (e.g, WebSockets) or retrieving data async (e.g., XHR).

mounirlamouri commented 11 years ago

If I understand correctly, the question here is whether we should do telephony.dial(...) or new TelephonyCall(...). This is a question we have been asking ourselves when we have been designing Web Activities, given that a Web Activity was simply returning a DOMRequest (ie. a Promise), we thought of starting an activity from navigator.something() and let the consumer handle the returned request. Finally, we found out that creating a constructor for the following interface was nicer for developers: interface Activity : DOMRequest. Technically, that was exactly the same thing but using new instead of a method call when using primitives is something developers seem to prefer.

This is not exactly the same context here but I think the basis stays: developers prefer using new Foo instead of calling a method that will return a Foo object. This design actually makes a lot of sense unless the method behaves like a factory and might return a different kind of object depending on the parameters - which is not the case here.

It would probably be good to introduce a constructor to TelephonyCall. This change does not seem very invasive (but I might miss something) so it can easily be tested and we could as well revert it if it happens to be a bad design.


Regarding the StateHandler idea, it looks quite close from the new EventTarget ideas that have been floating aronud recently. Annevk summarised them in this gist: https://gist.github.com/annevk/5238964

jsoref commented 11 years ago

First, constructors are the right way to go, because they improve documentability/discoverability as mentioned earlier.

I'm not a Futures expert, but I believe that the following addresses your use case above:

sim1call = TelephonyCall("0123456789", {service: "sim1"});
// Promise me that if i'm put on hold, you'll do X for me:
holdpromise = sim1call.ifheld();
holdpromise.then(onhold);
// The main promise:
callpromise = sim1call.dial();
callpromise.then(onactive, ondisconnected).then(writeCallHistory, writeCallHistory);
marcoscaceres commented 11 years ago

I think I found another problem where not having the constructor results in API inconsistency. Given:

navigator.telephony.call("I am not a number"); 

The above results in an Event being dispatched to the onerror event handler attribute, while other methods of the API will handle errors in a Promise.

Also, in the case where an event is generated on construction: if one tries to use this API in a JS console, the above will lead to a race condition, because the developer cannot register the onerror handler before the event is fired. Having the constructor allows the code to be run interactively. I don't know about others, but I run code interactively in the JS console a lot - and dislike APIs that don't allow me to do that.

anssiko commented 11 years ago

I think we may want to try code the use cases involving TelephonyCall against the API with and without a constructor, then test both the alternatives on actual web developers as @mounirlamouri mentioned Mozilla did for Web Activities. A cheap JS prototype would go a long way.

More generally, prototyping the [main] use cases against any API in code before polishing the spec fully may save us from specifying and implementing something that has issues (cf. IndexedDB).

@zolkis mentioned earlier he's working on a prototype implementation in JS, plus a simple dialer app. Just the dialer app would be very helpful. @slightlyoff did something similar for a bunch of APIs with and without Promises, see e.g. https://github.com/slightlyoff/Promises/tree/master/reworked_APIs/IndexedDB/example

Also @marcoscaceres has done some interesting experimentation with teleprolly.js https://github.com/marcoscaceres/teleprolly. Perhaps @zolkis and @marcoscaceres could work together to come up with something interesting, WDYT? :-)

zolkis commented 11 years ago

To be discussed on the F2F.

zolkis commented 11 years ago

Based on the F2F discussion, we leave the API as it is now, i.e. the dial() method creates a TelephonyObject. I still leave the issue open, since the 2-phase constructor idea has not been explored with yet. Besides, comment https://github.com/sysapps/telephony/issues/17#issuecomment-20201347 is valid.

marcoscaceres commented 11 years ago

WFM.

anssiko commented 11 years ago

Ditto.

zolkis commented 11 years ago

I am reconsidering this issue, and also experimenting with using constructors. See https://etherpad.mozilla.org/UVYBJXiOsP and (further development) https://etherpad.mozilla.org/cByqOalX3d