Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should writeProperty() return a value #193

Open
danielpeintner opened this issue Sep 10, 2019 · 38 comments
Open

Should writeProperty() return a value #193

danielpeintner opened this issue Sep 10, 2019 · 38 comments
Labels
for next iteration Planned or postponed topics for the future use case Describes a scenario that may be useful for technical decisions wait-for-td

Comments

@danielpeintner
Copy link
Contributor

We currently have the following definition for writeProperty()

  Promise<void> writeProperty(DOMString propertyName,
                              any value,
                              optional InteractionOptions options = null);

I wonder whether the return value should be Promise<any> instead of Promise<void> ?

There may be different use-cases were a server might modify the value initially provided (e.g., round a number 1.2345 to 1.23 by limiting digits) and returning the actually set value would be helpful. No additional readProperty() needed.

What are the consequences of such a change?

@zolkis
Copy link
Contributor

zolkis commented Sep 10, 2019

IMHO it looks difficult to make a consistent API contract/semantics here.

  1. If the semantics is "return the actually written value"
  • Some protocols/devices will just support an ack/error, not the value they have actually written.
  • So the implementation cannot promise to know the exact value written, unless read again, which is an extra roundtrip and then it's best done by the application script.
  • Returning the same value from implementation if there is no support to know the actual written value does not fulfil the semantics "return the written value".
  1. If the semantics is "return either the same value or the written value"
    That's worse than re-reading the value, so I would not go here.

  2. If the semantics is "return either the written value or undefined if not supported", (where undefined is represented as a special object reference in ECMAScript that denotes undefined).
    I guess in most of the cases this could be implemented., but this might lead to inefficient implementations for some of the unsupported cases and then the app scripts should have the control to manage that themselves and don't rely on implementation smartness.

This is mainly why the current spec just writes the value, reports success/error and if a script is interested in the real value written, would need to call a read again.
Note that if there is protocol support for returning the actual (potentially modified) written value, implementations could cache the actually written value and provide that without roundtrip on the next read request if certain conditions apply.

Speaking about that, we could have scripting-specific InteractionOption entries for reads, e.g.

  • making a new read request or cached value is fine within given reporting rate (or validity period)
  • ones that would support high frequency reads (e.g. sampling rate, reporting rate, buffer size etc), solving Very high frequency updates #107.

Then if we go there, we could have scripting-specific InteractionOption entries for writes, e.g.

  • preference to provide the actual written value if supported; if not supported, then null is returned.

So I'd go with the following solution: define a write InteractionOption flag the apps can set, but the default should be off, for efficiency. If the option is not supported for given device or protocol, instead of a write error, the write would succeed but it would return undefined for the written value. Then the app will know that an explicit read is needed.

However, the extra check needed for undefined makes this impractical in scripts, so IMHO it would be both simpler and more efficient to leave it the current way, requiring apps to re-read, eventually adding some read options outlined above.

@zolkis
Copy link
Contributor

zolkis commented Jan 20, 2020

We could indeed modify the writeProperty() signature to return any. But what would be the algorithm?

Ideally we'd need a parameter or an interaction option for telling "please return the written value if possible", with the default being false, in order to avoid unnecessary network round-trips.

That interaction option could (should?) come from the TD. Since the TD "owns" interaction descriptions, IMHO the Scripting API should not invent extra interaction options that are not supported by the TD.

As for the algorithm -- assuming there was that interaction option --: if the other end doesn't support returning the written value, it should return null, meaning the client has to make an explicit readProperty() request.

Corner cases:

  • if the Property was write-only, write_with_return would return null
  • if the Property was read-only, write would return error anyway.

@egekorkan
Copy link
Contributor

egekorkan commented Jan 20, 2020

I think it can help to think from the perspective where the write request is actually responded with a payload, how would the Scriping API handle it.

  • HTTP: Server shouldn't return anything in the body but it can indicate that the value hasn't changed, is not allowed or is successful IF writeproperty is done as a PUT request ( https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/PUT). If it is done as a POST, then it can return a body. (I think all these apply to CoAP as well)
  • MQTT: With MQTT 5 it is possible to indicate a topic to respond to (by publishing) for a publish request. So technically an MQTT Thing can also reply to a writeproperty request with a specific payload.

For the case of node-wot http binding, an Exposed Thing can reject the writeproperty and supply a string (like an error message) which will be automatically returned to the client. At the moment, if something like this happens, there is no way to describe it in the TD or intrepret it in the scripting level. This was the reason behind issue #200 .

@zolkis
Copy link
Contributor

zolkis commented Jan 20, 2020

Well, if a WoT implementation adds something to the interaction that has been defined in the TD, that is thin ice territory and subject of eventual future breaks, since the TD is supposed to be able to define the whole interaction, including error handling (and sensible defaults for the non-defined cases).

Of course, the Scripting API should be generic enough to allow that - we agree there.

The error path is already almost covered, since the data returned with an error could be conveyed in an Error object on the reject promise path. What we've been discussing in #200 was the error representation (mapping of error names, not the data). The error data could already been transmitted in my understanding.

The open thing (to me) is this issue, i.e. allowing a return value from writes, which it seems we could do anyway, based on your comment. What remains open (and discussed in TD issue 875 is the algorithm: what does the TD define and how Scripting should implement it.

@mmccool
Copy link
Contributor

mmccool commented Feb 24, 2020

If the implementation does something that is incorrectly described in the TD, then the TD is incorrect, not the implementation. The TD describes the implementation, whatever it does. So, if for example, we generate tests using the TD and find that we CAN write to a readonly property, then the test should indicate that the TD is an incorrect description.

@mmccool
Copy link
Contributor

mmccool commented Feb 24, 2020

Also, to follow up on "should a value be returned from a write": if we want to describe pre-existing implementations, we need to allow for the fact that some implementations may have writes that do NOT return the new value. In fact this might be done to save bandwidth; or the device might do something else entirely, like send a simple acknowledgement.

I think it's ok to say that the default behaviour is to echo the written value. However, we probably need a way to override that in the protocol binding (and this should be some normative attribute, which we currently don't have, so... probably need to think about adding this to TD-next; maybe "writeEcho" with a default value of "true". The other thing we can do in the meantime is just say that anything that does NOT echo data written should be described as an "action"... and this is fine in the short term, and maybe in the long term, too... but we should discuss further, maybe in the TD call).

@mmccool
Copy link
Contributor

mmccool commented Feb 24, 2020

Based on the discussion, returning "void" from writeProperty is also fine, and in fact is better, since it covers all cases (although it might ignore data from the protocol). If you WANT to look at the return data... then use an action.

@danielpeintner
Copy link
Contributor Author

In the Scripting API call on 2/24 we discussed the topic and the consensus was to stay with the current form of returning void in case of writeProperty() because it is

  • more consistent
  • simpler and
  • safer.

Moreover, for the case (or the need) of returning a value the action interaction is a better fit.

Note: We also plan to collect/log such design decisions in Github, the spec or an auxiliary document.

@zolkis zolkis closed this as completed in 3f431f4 Feb 26, 2020
@danielpeintner
Copy link
Contributor Author

Note: We also plan to collect/log such design decisions in Github, the spec or an auxiliary document.

see #205

@egekorkan
Copy link
Contributor

Actually, what is the expected behavior when a writeproperty indeed returns something? It could be that the TD is designed for an existing device and the TD cannot be changed due to some reason. Is it hidden to a Scripting API Consumer? There could be an issue (albeit a small one) if one interacts with that (weirdly designed) Thing via a REST Client and then with a Scripting API implementation and observe different behavior.

@zolkis
Copy link
Contributor

zolkis commented Mar 15, 2021

Actually, what is the expected behavior when a writeproperty indeed returns something?

Right now the Scripting API can handle it only if it that something is an error.
For positive payload returns an Action can describe the interaction, hence and action should be modeled in the TD.

If we want to blur the semantics between writes and actions, writeProperty() could return Promise<InteractionOutput> instead of Promise<undefined>.

But then why don't we simplify things (following the logic of the question) and just support a generic interaction API, that can be a read, write or action, depending on a parameter or context and/or a name-resolving algorithm? (e.g. if name resolves to a single interaction, use that, otherwise if there are conflicts, an extra InteractionOption would tell what the interaction was meant to be. One can already pass data in an InteractionOption, so inputs are covered. Then, we'd need a way to do multiple interactions in a single request: that could be told the implementation by a start- and end-transaction function, or by a signature for multiple inputs/outputs like we do now).

Promise<InteractionOutput> interaction(DOMString name,  optional InteractionOptions options = null);

partial dictionary InteractionOptions {
    InteractionInput input;
    boolean observe;
    InteractionType interaction;
};

enum InteractionType { "property", "action", "event" };

But we have already discussed this idea many times in F2F meetings and after, so let's draw a line somewhere :).

@zolkis
Copy link
Contributor

zolkis commented Mar 15, 2021

Even more simple: why don't we use a generic Request object, a Response object and the Fetch API and call it a day?
This has been suggested earlier many times. We could spare a lot of duplicated/WoTified algorithms and be more aligned with REST in the same time :).
But wait, that is already possible to do, and since Scripting API is optional in WoT, there are no conflicts :).

@relu91
Copy link
Member

relu91 commented Mar 15, 2021

For positive payload returns an Action can describe the interaction, hence and action should be modeled in the TD.

+1 and this goes along the discussion about when to use properties or actions (w3c/wot-thing-description#1020). As I understood Scripting API follows closely the interaction model defined in architecture and I think we should strictly follow it otherwise, as @zolkis is suggesting, we end up creating yet another fetch API. At the same time, the Interaction Model does not really specify what a write operation on a Property should do:

A Property is an Interaction Affordance that exposes the state of the Thing. The state exposed by a Property MUST be retrievable (readable). Optionally, the state exposed by a Property MAY be updated (writeable). Things MAY choose to make Properties observable by pushing the new state after a change (cf. Observing Resources [RFC7641]). Write-only state should be updated through an Action.
If the data is not fully specified by the Protocol Binding used (e.g., through a media type), Properties MAY contain one data schema for the exposed state.
Examples of Properties are sensor values (read-only), stateful actuators (read-write), configuration parameters (read-write), Thing status (read-only or read-write), or computation results (read-only).

So maybe we should improve the text there.

@egekorkan
Copy link
Contributor

For positive payload returns an Action can describe the interaction, hence and action should be modeled in the TD.

So I also somewhat agree with this, when a property write (especially PUT) return some payload, there is something wrong in the design. However, there are cases that this happens. TD section written above is underspecified maybe but based on the text above, nothing stops someone from writing a property called configuration that returns a custom object. I like the proposal above from @zolkis of having a generic output/response object. Since this is a corner case, it should not cause trouble but it sort of does, sorry about that :D

@relu91
Copy link
Member

relu91 commented Mar 15, 2021

So I also somewhat agree with this, when a property write (especially PUT) return some payload, there is something wrong in the design. However, there are cases that this happens.

Wandering, can we just suggest the TD designer make the property read-only and add an action to set that property?. Since it is also advised that actions may change properties I think this should be the right way to describe that API with a TD.

@egekorkan
Copy link
Contributor

Yes but if you are interacting with an existing Thing with an existing TD that you cannot change, you don't have the option to do that. Of course I can write a TD that mirrors and adapts according to my wish but that would be annoying to manage :D

@zolkis
Copy link
Contributor

zolkis commented Mar 15, 2021

TD's should be well specified, validated and fixed time to time... meanwhile these misdesigned TDs can be still handled within the specs. Do we have examples on what exactly is missed if an app ignored response data from a Property write?

@danielpeintner
Copy link
Contributor Author

The discussion whether a writeproperty can return a value seems to be still ongoing..

see w3c/wot-profile#77 (comment)

FYI: That's why I reopen it...

@danielpeintner
Copy link
Contributor Author

The outcome for the profile seems to head towards returning 204 without any body content.

Having said that, should we still try to allow reporting a body for writeproperty ? It seems people seem to expect it..

@danielpeintner
Copy link
Contributor Author

Scripting API Call 2021-05-10

  • TD should properly design whether interactions have input/out
    • readProperty() have input (e.g., uriVariables)
    • writeProperty() output value
  • What about responses that may return a different dataSchema in a PUT ?
    • e.g., Philips HUE
    • grafik

@egekorkan
Copy link
Contributor

So one thing that needs to be specified, that can apply to other operations as well, what happens if the Thing behaves differently than how it is described in the TD. Since a return value is not describably in a TD, if there is such a return value we would be in the case of a Thing not behaving according to its TD.
In the case of a Thing returning a payload for a writeproperty, currently a Consumer application can:

  1. Not communicate it to the Scripting level
  2. Give a certain predefined error
  3. Cause an exception, forcing the Consumer application to handle this in the Scripting level.

I think that node-wot does option 1 at the moment.

P.S.: When I said that can apply to other operations as well, this can happen with subscribeevent, unsubscribeevent, observeproperty, unobserveproperty as well.

@zolkis
Copy link
Contributor

zolkis commented May 10, 2021

I admit being a culprit for this limitation. It would not be difficult to allow return value :), but my main reasons why Scripting does not support returning values from writes is the difficulty to guarantee consistent behavior across various use cases.

The semantic contract for writing a property with a value is success (value written) or failure to do so.
There are the following cases:

  • In most cases, reporting back the same value is not needed (it was the app who provided it).
  • In some cases, reporting back the value would mean sending back a lot of data, with no actual gains.
  • When someone else can also write that value during the transaction, the app should rather set up an observer.
  • When the remote end stores a different precision than provided, it's disputable (and the strongest use case for supporting a return value), but that could be handled by the data schema, which could contain the precision, so the app knows what can be stored. Doing a round-trip just for that reason, and inferring precision from a write operation is a bad design and false sense of security IMHO. I would not base my app logic on such comparisons. Also, it cannot be changed by the app, only handle it, but then it could handle it anyway (e.g. display the value that is read). For doing local calculations, I fail to see the need to limit local precision because of remote precision.

So the intent with not providing return value is to give incentives for good designs and avoid complications on implementation and application level.

However, we are open to re-discuss this for use cases that justify this.

@egekorkan
Copy link
Contributor

I also heavily think that we should not encourage this.

The use case above is from Philips Hue and if I use a property to describe that HTTP request, I cannot describe the response of the write request. In our case, we simply used an action to model that since it doesn't matter for us that an action is done via PUT. However, someone can model that as a property. In that case, no matter what we think, the device is going to respond with a payload. Thus, the behavior on the scripting level should be defined. Even saying that Scripting API does not support that, you will maybe have an exception, maybe an error, maybe X, thus your scripts should handle that somehow is fine by me.

P.S. I find the Hue API really ugly, if someone from Philips sees this, well, they should see this ^^

@zolkis
Copy link
Contributor

zolkis commented May 11, 2021

We can make a Note in the spec warning about this and eventually pointing to this issue.

The TD defines interactions for a reason and developers should be aware of what interactions are meant for.
If someone wants to model a thing that returns a value from a write, use an Action. In the Philips Hue case above, input is an object and output is an object. That is because requests seem to be bundled at application level. Error handling as well is buried in the success-case (the apps has to parse each element separately, some of which may be success, others errors) - which impresses as a brain-dead design, but maybe they have a reason for it. Maybe it's their version of being protocol-invariant.

As far as I see it, the use cases are covered and I would not like to mess up the API, the spec and the implementation just in order to have a property-like-action and an action-action :)

Please tell if I misunderstood something, but in my reading since there exists a TD that can represent Philips Hue things, I'd say we should be fine. It would be hair splitting to criticize why something is represented as an action instead of a property, as long as things work.

@egekorkan
Copy link
Contributor

I totally agree with the points above, however, we cannot control Thing developers and TD designers. From the comments above, I still do not understand what the behavior of a Scripting API implementation should when a Thing does return a value. I would say that firing an exception can be OK. A note should be added in any case but pointing to this issue does not solve anything since we do not have a resolution in this issue thread.

@danielpeintner
Copy link
Contributor Author

danielpeintner commented May 13, 2021

FYI:

  • ECHONET seems to report a body value in case of PUT (see page 20 of slides)
  • OCF CoAP

see also w3c/wot-profile#77 (comment)

@zolkis
Copy link
Contributor

zolkis commented May 14, 2021

From the comments above, I still do not understand what the behavior of a Scripting API implementation should when a Thing does return a value. I would say that firing an exception can be OK.

That depends on the situation, see below (Echonet). In most cases I argue the implementations can handle it:

  • If the returned value matches, resolve.
  • If it does not match, then if the difference is within the tolerances/range specified by DataSchema, it is okay, resolve.
  • Otherwise reject the write with an error.
    But if it returns a totally different object, I agree it should be rejected since that should have been modeled as an Action.
    The developers should check and use a better TD in that case.

A note should be added in any case but pointing to this issue does not solve anything since we do not have a resolution in this issue thread.

Since it's an open issue, it won't have a resolution, but provides the context of discussion, enabling participation.

FYI: ECHONET seems to report a body value in case of PUT

Nothing wrong with that, but in general it should be modeled as an Action in a TD.
However, in that particular case the implementation can encapsulate it, seeing it's reported back the same value, so it's OK to resolve the write to the script.

@zolkis
Copy link
Contributor

zolkis commented May 18, 2021

OCF CoAP

FWIW in OCF a write is modeled as an update via POST, and the other end needs to reply with success (2.xx) or error (4.xx or 5.xx). For create with POST, the same. This is in the OCF core spec. However, I've seen swagger files that allow data to be returned on POST (with response/200), and there is no limitation against that for error responses, either.

In most cases (OCF resource type spec, Device spec) the data returned is the same as the one sent for writing. So that one could be encapsulated by WoT implementations (there is no point to return obvious data to the script).

Also note that "range", "step" and "precision" are OCF resource property definitions on base resource types (all that participate in CRUDN), so endpoints have a way to know this about the resources they are using. So IMHO the TD spec should be able to describe these for Properties.

I have not seen data returned on errors.

@mjkoster could you please confirm on

  • how widely the pattern of returning data on writes is used in OCF devices/resources, per success and per error cases;
  • when used, what is the main use case: precision/data representation (I doubt that), or choices on error path (e.g. send back available value options), or .. (what else)?
  • for the precision use case, could the API implementation handle/encapsulate it, or should the app handle it, or should the TD spec address the topic of precision?
  • or should the app be able to give a desired tolerance/precision for data storage (like most physics experiments must do), which would make it possible for the API implementations to encapsulate it (i.e. only return success or error). This would not solve the use case when options are sent back (for error resolution).

The problem is inconsistency: the TD spec provides abstract interactions, and currently only Actions can be used for an interaction that returns data. So TDs that try to model things that return data on writes will look awkward: writing an OCF property could be represented as a Property write (if no data is returned) or an Action (if data is returned). The latter would blow up the structure of the TD (a property is represented under both "properties" and "actions"). Such a TD would "look better" if writes could optionally return data. So IMHO the TD spec should allow returning data on writes.

In summary, IMHO the TD spec should address these:

  • can write interactions return data, on the success and/or error paths? how that data is represented (by DataSchema/content-type)?
  • can any interaction return data on error path, and how are these represented?
  • model precision [and step] for properties in DataSchema, as range is already supported.

@danielpeintner
Copy link
Contributor Author

danielpeintner commented May 19, 2021

The problem is inconsistency: the TD spec provides abstract interactions, and currently only Actions can be used for an interaction that returns data.

sidenote: I always thought the same but I did not find a statement in the TD spec that prohibits to return any value for writing properties. At least the TD spec is silence about it and and one could think of reporting the same dataSchema is OK. The TD spec should clarify the expectation. What is clearly not allowed for properties is reporting any value other than the original dataSchema.

EDIT: I created w3c/wot-thing-description#1146

@relu91
Copy link
Member

relu91 commented May 19, 2021

Also, I would like to know how these systems that return the value behave if I am writing something that is not plain json. Do they still return the file back? (i.e., image, video, etc.).

@egekorkan
Copy link
Contributor

But if it returns a totally different object, I agree it should be rejected since that should have been modeled as an Action.
The developers should check and use a better TD in that case.

So in Echonet, they think of actions as a more special case of affordance. They actually use the term property in the slides for the PUT operation.

In any case, the issue persists when a Consumer, who does not control how a TD is written, gets a TD where the writeproperty returns a value that is completely different. We cannot expect Consumers to swap properties for actions when they don't think that it is correct.

@zolkis
Copy link
Contributor

zolkis commented May 20, 2021

In any case, the issue persists when a Consumer, who does not control how a TD is written, gets a TD where the writeproperty returns a value that is completely different.

That might be considered an error in Scripting, or just ignored (when a success code is reported), which is why the TD spec should specify write ops in more details.
In general, what happens if a consumer consumes a buggy TD? They will likely get errors. What can the consumer scripts do about it? Look for a better TD of that thing (in the free market, or from the thing manufacturer etc).

I am not really supporting the idea of allowing a write return just about anything, it's what actions are for, and it's confusing why do we call that a "property write" after all. But there could be other use cases, as outlined above. So this issue needs to be discussed in the TD task force, update the TD spec, then come back here.

@danielpeintner
Copy link
Contributor Author

danielpeintner commented Aug 2, 2021

Scripting Call 2021-08-02

  • @relu91 fear that a possible optional return value causes questions in developers mind. Not sure what they should do with it...
  • @relu91 what IF just one form reports a value (i.e. given form index)
  • @danielpeintner existing frameworks that do report return values
  • @ashimura profile work should specify (see New Protocol Binding section wot-profile#77)
  • Philips HUE, example to be added, see example TD that uses an action setState that uses "htv:methodName": "PUT" given it is a HUE property.

@egekorkan
Copy link
Contributor

Philips HUE, example to be added

@danielpeintner should I provide one?

@danielpeintner
Copy link
Contributor Author

Philips HUE, example to be added

@danielpeintner should I provide one?

Yes, I wanted to reach out to you 👍

@egekorkan
Copy link
Contributor

egekorkan commented Aug 2, 2021

Well actually the example is already there: #193 (comment)

This can be either modeled as an action or property. I chose action for the sake of simplicity in my TDs.

@relu91
Copy link
Member

relu91 commented Aug 9, 2021

We discussed this topic in the TD call last week. It seems the consensus went on introducing this feature in the TD document. As far as we are concerned this feature will impact different aspects of our APIs, not only the returned type of writeProperty operation but also how runtimes interpret the data schema provided in an Interaction Afforandace.

@JKRhb JKRhb added for next iteration Planned or postponed topics for the future wait-for-td labels Dec 11, 2023
@relu91 relu91 added the use case Describes a scenario that may be useful for technical decisions label Jan 15, 2024
@danielpeintner
Copy link
Contributor Author

There seem to be 2 issues in the TD repo taking care of it.

Not sure if we should close one of it. Anyhow, this might be part of the TD work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for next iteration Planned or postponed topics for the future use case Describes a scenario that may be useful for technical decisions wait-for-td
Projects
None yet
Development

No branches or pull requests

6 participants