Open relu91 opened 4 years ago
First of all I would use constructors.
Then, for a single onerror
the usual way is including it directly, instead of creating inheritance.
A variant of the above:
[Exposed=Window, SecureContext]
interface EventSubscription {
constructor(DOMString name, optional InteractionOptions options = null);
undefined start(); // could be left subscribe(), but this will look better in code examples
undefined stop();
attribute EventHandler onevent;
attribute EventHandler onerror;
// I don't think we need these, I would use Promise for start() and eventually stop()
attribute EventHandler onunsubscribeevent;
attribute EventHandler onsubscribeevent;
};
[Exposed=Window, SecureContext]
interface PropertyObserve {
// same stuff!
};
So in the end, we could merge the two kind of subscriptions to be handled by one interface,
[Exposed=Window, SecureContext]
interface Subscription {
constructor(DOMString name, optional InteractionOptions options = null);
Promise<undefined> start();
undefined stop();
attribute EventHandler ondata;
attribute EventHandler onerror;
};
[Exposed=Window, SecureContext]
interface EventSubscription: Subscription;
[Exposed=Window, SecureContext]
interface PropertyObserve: Subscription;
There are reasons why stop() is not with a Promise, discussed extensively in other specs. Code examples:
subscription = new EventSubscription("temperature");
subscription.ondata = interactionoutput => { ... };
subscription.onerror = error => { ... };
await subscription.start();
...
subscription.stop();
observe= new PropertyObserve("temperature");
observe.ondata = interactionoutput => { ... };
observe.onerror = error => { ... };
await observe.start();
...
observe.stop();
However, since the algorithms are different, we could specify the two kinds of subscriptions by duplicating the IDL (not a big deal).
Alternatively, we could go in the middle ground between this and your proposal, have start() and stop() return undefined
and include the event trinity from Generic Sensors, onactivate
, onreading
, onerror
.
But in the end I think the simplest would be to slightly modify the existing spec, re-introduce unsubscribeEvent()
and unobserveProperty()
(for the reason they have op
words in the TD spec and we align to that), and eventually use subscription id's (TBD).
About the Promise-onerror duality. I think we need to distinguish between errors like SecurityError (coming from runtime policies) that appear during setting up the subscription process and the underlying protocol errors that might pop up during the operating subscription.
IMHO we just need to be clear in the algorithm how they are handled. The Promise fulfills when the subscription is successful. The error event (or callback) could be used for all the opaque protocol errors. I see this as clarity and not as clutter.
First of all I would use constructors. Then, for a single onerror the usual way is including it directly, instead of creating inheritance.
The reason because I introduced the AffordanceHandle
was for extensibility. It helps if we'll ever have an ActionHanlde
. The onerror
event was the first thing that I found in common between the different interfaces, but I imagine that also other internal slots might be shared (i.e. Form, InteractionOptions, AffordanceName, etc.). Sorry I did not mention directly in the proposal.
Constructors might be fine, but we need to pass also a ConsumeThing
instance, for obvious reasons:
interface EventSubscription {
constructor(DOMString name, ConsumeThing thing,optional InteractionOptions options = null);
// Other stuff
}
I used different classes to have different names for events and functions. I thought it was clearer and closer to the Thing Description interaction model. Although, one can say that it is better to use a single API like you are proposing.
There are reasons why stop() is not with a Promise, discussed extensively in other specs.
So in your API we are missing a way to know when you are successfully unsubscribed/unobserved. I guess we should introduce an event.
So in the end it would be something like this:
[Exposed=Window, SecureContext]
interface Subscription {
constructor(DOMString name, ConsumeThing thing,optional InteractionOptions options = null);
Promise<undefined> start(); //no direct mapping with what is happening at the platform level (i.e. form with op subscribe/observer)
undefined stop();//no direct mapping with what is happening at the platform level (i.e. form with op unsubscribe/unobserver)
attribute EventHandler ondata;
attribute EventHandler onerror;
attribute EventHandler onstop;
};
[Exposed=Window, SecureContext]
interface EventSubscription: Subscription;
[Exposed=Window, SecureContext]
interface PropertyObserve: Subscription;
Even if I still not convinced that start should be a Promise. We are again mixing two different programming styles 🤔 .
But in the end I think the simplest would be to slightly modify the existing spec, re-introduce unsubscribeEvent() and unobserveProperty() (for the reason they have op words in the TD spec and we align to that), and eventually use subscription id's (TBD). About the Promise-onerror duality. I think we need to distinguish between errors like SecurityError (coming from runtime policies) that appear during setting up the subscription process and the underlying protocol errors that might pop up during the operating subscription. IMHO we just need to be clear in the algorithm how they are handled. The Promise fulfills when the subscription is successful. The error event (or callback) could be used for all the opaque protocol errors. I see this as clarity and not as clutter.
Subscription ids feel a lot like File Descriptors... can't we use the functions passed as callbacks as identifiers? 🤔
So in your API we are missing a way to know when you are successfully unsubscribed/unobserved.
We could have a Promise on stop()
, that is cheap, but it's only a syntactic thing that looks like a solution. Usually cancellation cannot be guaranteed in all cases. Suppression is a workaround, of course, but if we are exact, the function cannot guarantee the result reported, max simulate/fake it in the worst cases.
We are again mixing two different programming styles
Yes, because each is used for its designated purpose. The Promise is meant for long-ish operations that can either succeed or fail. A network request, for instance. Like subscribe request. Then, the error event is meant for notifying about operational errors during a longer operation, like the lifetime of a subscription. IMHO the mix-up would be if we didn't separate the errors for the subscribe request, and for the errors that might raise during the subscription lifetime. Anyway, a single error event is doable.
However, we have a long history/debate about avoiding events in the API design, because the EventTarget - Node EventListener controversy, and it goes back for many years. For this iteration, I would stay with callbacks: clean and clear, no uncertainty what actually is implemented from EventTarget in a Node or in a resource-restrained WoT runtime environment. Callbacks are far more efficient, if they don't blow up the API (too much).
About subscription id's: yes, we can use the functions as well, but IIRC @danielpeintner had an argument against that and preferred id's.
//no direct mapping with what is happening at the platform level (i.e. form with op subscribe/observer)
I don't understand that comment, since the current algorithms will all remain, i.e. it is very much connected to the forms. Stopping already has a clause for suppressing after unsubscribe/unobserve.
However, we have a long history/debate about avoiding events in the API design, because the EventTarget - Node EventListener controversy, and it goes back for many years. For this iteration, I would stay with callbacks: clean and clear, no uncertainty what actually is implemented from EventTarget in a Node or in a resource-restrained WoT runtime environment. Callbacks are far more efficient, if they don't blow up the API (too much).
FYI Node 14 has native experimental support for EventTarget and Events: https://nodejs.org/api/events.html#events_eventtarget_and_event_api.
Of course, there is also a polyfill library, although not so popular. On the other hand, I have to admit that callbacks might be easier (more efficient maybe?) to implement on constraint devices runtimes. We don't have to forget that we are designing APIs also for these devices, you're right.
I don't understand that comment, since the current algorithms will all remain, i.e. it is very much connected to the forms. Stopping already has a clause for suppressing after unsubscribe/unobserve.
I meant that a start()
method does not immediately evoke the fact that a subscribeevent
op will be called, whereas using subscribe()
is more direct.
I meant that a start() method does not immediately evoke the fact that a subscribeevent op will be called, whereas using subscribe() is more direct.
Ah, that is true, yet it's quite direct 1:1 mapping, so it should not be a problem.
However, I would prefer if we found a solution using ConsumedThing::subscribe().
I think we should re-evaluate the design proposal with the current approach. I think several proposes have been blended together. If we don't find any issue with the current design I think we can close this.
As requested here I wrote a new IDL for yet another variant of the subscription API. Please see the following Web IDL:
The design idea is to re-use as much as we can what is already out there. Therefore, here we are using
EventTarget
andEventHandler
.Pros
actions
. For example, if we find a way to define long-standing actions in the TD, we could define anActionHandle
observeProperty
,onevent
)Cons
subscribe
,unsubscribe
,observe
, andunobserve
(i.e. you can'ttry {}catch{}
but you have to use theEventHandler
What do you think? 🤔