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

Extended return type of invokeAction() #555

Open
danielpeintner opened this issue Jun 3, 2024 · 14 comments · May be fixed by #561
Open

Extended return type of invokeAction() #555

danielpeintner opened this issue Jun 3, 2024 · 14 comments · May be fixed by #561
Assignees

Comments

@danielpeintner
Copy link
Contributor

Based on the discussion in eclipse-thingweb/node-wot#1279 two points/issues arose. One is the following.


At the moment the invokeAction(DOMString actionName, ...) call may return InteractionOutput only. Doing so makes it difficult to properly handle long running actions that MAY report first an "ActionStatus" object that allows for cancelling/querying action.

The idea/proposal is to enhance the return type. Something like

Promise<InteractionOutput | ActionStatus > invokeAction(DOMString actionName, ...)

We need to define what ActionStatus might be and what can be supported.

I wonder whether we can define/collect fist ideas how ActionStatus could look like and we could also start experimenting with it implementation- wise. Is it possible to "label" such a feature "Experimental" so that it is clear it might change?

Note: The EXI Profile has also an ActionStatus object and we should make it clear that it is something else. Maybe we might also use another name...

realtes to #335

@zolkis
Copy link
Contributor

zolkis commented Jun 3, 2024

That would not be a good design - how do you write use code, to use both?
Actions should then return a control object that can control the action, including intermediary commands (state, pause, resume, stop, ...), final state/value reporting etc.

@lu-zero
Copy link

lu-zero commented Jun 3, 2024

we have this cascade:

  • invokeAction may return a success or a failure
  • success may be the affordance output (if described) or any of the additionalResponse of the kind success
  • failure may be any additionalResponse of kind failure
  • it could be an unexpected lump of data marked as success or failure according to the protocol

it can be modeled as a Result<AnySuccess, AnyFailure> and then the user has to see which kind of failure or success to expect or Result<AffordanceOutput, Result<AnySuccess, AnyFailure>> if we assume the AffordanceOutput is what we care about the most.

Given that ActionStatus will return the InteractionOutput one might just always get an ActionStatus, but it is an open problem what to do with additionalResponses of success kind.

@zolkis
Copy link
Contributor

zolkis commented Jun 3, 2024

To me it seems it's not a plain response path, you may have business logic dependent on the return patterns, therefore it is best exposed as an interface of its own, serving the results/errors.

If we'd have one-shot responses, then we could use Promises to track them, otherwise we need events to report state, and success / error information.

So the flow seems to be:

  • when action is invoked, it returns a Promise with a control object or failure (of the exception type, security etc, not because of the affordance)
  • the control object MAY right away be in a finished success state, with an AffordanceOutput exposed via property / getter and additional info
  • or a finished error state with error information / additional responses
  • or the state may be progressing, in which case there could be methods on the object that could be used (depending on the Action type).

@danielpeintner
Copy link
Contributor Author

To continue the discussion I would like to summarize what has been discussed so far.
Based on code we are talking about something like this, right?

Promise<ActionControl> invokeAction(DOMString actionName, ...) 

interface ActionControl {
   state; /* enum of success, error, running, ..*/
   error;   
   getOutput(); /* returning InteractionOutput IF state is in success */
   cancel();
   ...
}

How to we deal with additionalResponses? BTW, we don't have support for it in the case of property also. Maybe we need to re-think it a bit in a broader sense.

What I don't like about the proposal:

  • we need to have statements like getOutput() available if in success state only etc
  • breaks all current implementations that expect InteractionOutput
  • ...

@JKRhb
Copy link
Member

JKRhb commented Jul 8, 2024

breaks all current implementations that expect InteractionOutput

Hmm, I wonder if it could make sense to introduce a separate method for only invoking asynchronous actions? Or would that be overkill?

@danielpeintner
Copy link
Contributor Author

Hmm, I wonder if it could make sense to introduce a separate method for only invoking asynchronous actions? Or would that be overkill?

Mhh, I don't think this can work out in all cases.

The synchronous term of an action is first of all optional (Hence, a developer does not really know which method to call). Moreover, I don't know whether we can/should assume that an asynchronous action cannot return right away (maybe that's just me that thinks so 🙈).

Anyhow, I think it is much more flexible that the entry point of an invokeAction() is the same and the server can decide what you will get on the client side.

@zolkis
Copy link
Contributor

zolkis commented Jul 10, 2024

What I don't like about the proposal

These are valid. But the TD spec has changed in this regard since the current API was designed, so the break was warranted.

We need to update the API to properly model the interactions, rather than patching it up.

In order to keep backward compatibility, I wonder if extending the InteractionOutput interface with state, error, cancel() would be enough, together with spec prose plus algorithmic steps controlling valid usages of the object, for instance depending on the state and the source, data could be already fetched from a stream as the action is progressing, but in other cases would give an error and the script would need to wait until the state becomes finished. Scripts might also want to register listeners to various states.

Therefore I suggest we experiment with extending the InteractionOutput interface with a state property and a control object that might contain methods depending on the state (e.g. cancel() that would only work when the state is running).
Additional responses could be modeled by an array, or by conflating the various responses by the implementation, following the TD (we need to specify algorithms for this).

When the state is success, the interaction output would represent a (simple or compound) value according to the TD (schema, form, data), like now. The control object could be null in the returned InteractionOutput.

When the state is error, the interaction output would represent a single or multiple error codes, according to the TD (schema, form, data). The control object could be null in the returned InteractionOutput.

When the state is running, the control object would expose cancel(), progress (or percentage) etc. Invoking cancel() in a finished state would result in an error or nothing (depending how we spec it).

@JKRhb
Copy link
Member

JKRhb commented Jul 11, 2024

In order to keep backward compatibility, I wonder if extending the InteractionOutput interface with state, error, cancel() would be enough

I think that sounds pretty good actually and should also fit in quite well with the current shape of InteractionOutput since value() and arrayBuffer() return promises anyway. So an implementation that uses the current API could still receive the data asynchronously, just without the ability to query the action state or cancel it.

Would it make sense to create something like an ActionOutput interface then that inherits from the InteractionOutput interface and adds these additional fields/methods? Or should InteractionOutput be extended in general to also cover aspects like additional responses for the other affordance types?

@zolkis
Copy link
Contributor

zolkis commented Jul 11, 2024

Both could work, subclassing is cleaner, we can discuss these. Also need to check the other interactions.

Error could expose an array of { error code, message and time stamp}, if error history is desired.

@danielpeintner
Copy link
Contributor Author

danielpeintner commented Jul 11, 2024

This seems heading towards a nice direction 👍

I would favor extending InteractionOutput with ActionInteractionOutput. I think doing so offers a good fallback for existing solutions.

@relu91
Copy link
Member

relu91 commented Jul 22, 2024

I like extending InteractionOutput with ActionInteractionOutput too. I think we can start to bake a PR and discuss the details of the algorithms there.

Additional responses could be modeled by an array, or by conflating the various responses by the implementation, following the TD (we need to specify algorithms for this).

Consider that even if TDs specify multiple success answers, clients always get at most one success answer. At least this is how I interpret the current specification. The same goes for errors.

About cancel() feature we could make it to solve also this issue: #449

@TallTed
Copy link
Member

TallTed commented Jul 22, 2024

Copy and paste from here or here (or quotes thereof) may lead to protracted lifespan of a couple of typos.

InteractionOuput and ActionInteractionOuput should be
InteractionOutput and ActionInteractionOutput, respectively.

@danielpeintner
Copy link
Contributor Author

Copy and paste from here or here (or quotes thereof) may lead to protracted lifespan of a couple of typos.

InteractionOuput and ActionInteractionOuput should be InteractionOutput and ActionInteractionOutput, respectively.

Yes, sorry 🙈 ... fixed the occurrences...

@zolkis
Copy link
Contributor

zolkis commented Jul 23, 2024

When designing API [extentions], we should also take into account the TD action affordance, i.e. whether it's safe, idempotent, and synchronous.

Also, additionalResponses in a form are a complication for the API. The intent described in the TD spec confine the usage to reporting multiple success codes or errors.
IMHO these could (and should) be encapsulated and managed by the implementations, and exposed only the success and error values to scripts, not the additional responses as interactions themselves.

If the affordance can receive updates after completion, and scripts want to handle the updating process themselves, then we need to expose it via the API. But then an event / subscription based design is needed with multiple InteractionOutputs provided, and it's a departure from the current design. IMHO that would be an overkill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants