WIP fix: issue https://github.com/eclipse/thingweb.node-wot/issues/230 ?

[danielpeintner]

remove resolve

[danielpeintner]

Why does Travis not run the tests for this PR?

[egekorkan]

Could it be also fixing #202 ?

[egekorkan]

It seems to actually run but not display here.
See: https://travis-ci.org/github/eclipse/thingweb.node-wot/builds/691230961

[danielpeintner]

Could it be also fixing #202 ?

Mhh, @franckOL reports that it causes another issue or did I misinterpret the comment

I try to remove resolv, and it works, of course i have now a perpetual unresolved promise (but catching polling error that happens)

[relu91]

IMHO, the subscribe function should resolve if the subscription was successful. Any other error that happens after that moment, should be communicated to the subscriber through the listener interface.

[franckOL]

IMHO, the subscribe function should resolve if the subscription was successful. Any other error that happens after that moment, should be communicated to the subscriber through the listener interface.

Agree with you @relu91 , but listener not notify about any error

[franckOL]

IMHO, the subscribe function should resolve if the subscription was successful. Any other error that happens after that moment, should be communicated to the subscriber through the listener interface.

in fact (for http binding, mqtt), what I see is that the promise is resolved after the first event triggered, and it can failed if no event emited for 60 * 60 * 1000 seconds (one hour).

[danielpeintner]

@relu91 @franckOL may I ask you to provide a proposal in "code" to better understand what you mean.

The listener can only have data/value passed?

[relu91]

One possible solution could be to change the WoTListener signature like the following:

type WotListener = (data: any,error?:any) => void

and in subscribeEvent :

return client.subscribeResource(form,
                    (content) => {
                        if (!content.type) content.type = form.contentType;
                        try {
                            let value = ContentManager.contentToValue(content, <any>te.data);
                            listener(value);
                        } catch {
                            listener(null,new Error(`Received invalid content from Thing`));
                        }
                    },
                    (err) => {
                        listener(null,err);
                    },
                    () => {
                        resolve(); // I am not sure that we need to call resolve here... 
                    }
                );
            }

However, we probably need to make a more drastic change. From the specs the subscribe method should resolve when the underlying connection is established. Consequently, we might change the client.subscribe function to return a Promise that resolves when it creates the Subscription. What do you think?

[danielpeintner]

One possible solution could be to change the WoTListener signature like the following:

type WotListener = (data: any,error?:any) => void

Correct, but this would require a change in the Scripting API (see https://w3c.github.io/wot-scripting-api/#idl-index)
Not sure if we want to do so...

[jak0bw]

One possible solution could be to change the WoTListener signature like the following:

type WotListener = (data: any,error?:any) => void

Correct, but this would require a change in the Scripting API (see https://w3c.github.io/wot-scripting-api/#idl-index)
Not sure if we want to do so...

I am not really sure if there is a choice regarding this change in the Scripting API. I see two main problems with the current implementation/design:

1. The scripting api specifies in §7.14 7-9 that subscribeEvent() should make a request to the underlying platform (7) and reject when an error occurred (8) and resolve otherwise (9). This seems not to be exactly the current behavior of node-wot. If the request is successful but no event occurs node-wot just does not resolve (probably until the timer is hit after one hour). This behavior is maybe not a strict contradiction to (9) but at least very unintuitive for the programmer and thus should be documented.
2. The main problem I see with the current implementation/design of node-wot/scripting api is how event problems are handled after a first unproblematic event emission occurred. The subscribeEvent() Promise is already resolved after receiving the first event -> it can not reject the error. With the callback function having no interface for errors there seems to be no way of handling this kind of errors or at least communicate this errors to the application consuming the thing.

The solution to both of this problems would be the proposed change to WotListener and is therefore required?

[zolkis]

Protocols differ from (JavaScript) programmatic interfaces in what concerns subscriptions vs error management.

Usually event subscriptions (in SW) are for events (i.e. positive outcome): whenever the event is triggered, the subscribers are notified. The subscriber is interested in the events.

Therefore, the subscribe() method succeeds when the subscription is recorded, i.e. everything is configured to that the server can sent events. If that is not possible, the subscription is rejected with an error.

Usually when errors occur, there are separate events for conveying those errors. Usually there is one onerror event used for that. This is a matter of TD design, not Scripting API design: if a Thing wants to report errors, it can define a SW event for that, including information about the protocol event that went wrong.

If the Scripting API would mandate an event onerror that is not defined by a TD, then what should be the behaviour? Never fire if not supported by the Thing?

[zolkis]

IMHO, the subscribe function should resolve if the subscription was successful.

+1

Any other error that happens after that moment, should be communicated to the subscriber through the listener interface.

That is also an option. However, traditionally (if one can speak of such in JS) errors are communicated via an error event, i.e. if the protocol signals error, the implementation fires the onerror event with the error that describes the protocol error. In normal course of events the implementation calls the Event subscriber callback with the data associated with the Event, always with data and not with error.

How can the app know where the error belongs? From the error content the implementation generates. We need to define an error for this in the Scripting API if we want to support the use case.

[egekorkan]

I think that we can start by thinking of the following cases:

• Error while subscribing: Something prevents the subscription, no event data comes. Examples could be wrong url for SSE and longpoll in HTTP, broker disconnecting (or never available) while subscribing in MQTT, a WebSockets subprotocol that has specific error type for this, or wrong data supplied in the subscription payload.
• Error while waiting for an event: Examples would be broker disconnecting in MQTT, Thing closing port in SSE and longpoll (in longpoll the error could happen while waiting for the response of the GET request or while trying to send the subsequent GET requests)
• Error while unsubscribing: Examples would be a WebSockets subprotocol that has specific error type for this, or wrong data supplied in the cancellation payload. I guess broker disconnecting would be a successful unsubscribe for this :)

All these can be mapped to operations of Consumed Thing interface or Listener interface. It would be interesting to think if there can be other cases.

[danielpeintner]

I close this PR unmerged since the new API is coming also and makes the effort obsolete ...