432 Added the ability to return data from a raised intent

[kriswest]

resolves #432
Adds the ability for apps resolving intents to also return data that may be retrieved from the IntentResolution by the app that raised the intent.

• Introduces an IntentHandler type, similar to ContextHandler, but with an optional return type of Promise
• Adds a getData() function to IntentResolution
• Removes the deprecated data field of IntentResolution
• Adds details and examples to the spec and API reference

A different PR will deal with adding support for specifying the desired return type when resolving an intent through the API and for adding metadata to do so to the AppD.

[nkolba]

@kriswest great to see data intents being picked up again! A lot to think about here, but my first question would be why change out the data property for the getData method? raiseIntent returns a promise, so not clear what the extra async call buys us. Beyond adding an extra call, downsides I'd be concerned about would be:

• the desktop agent now has to maintain state to pass context back with an intent, as they will need to hold the return context for that specific raiseIntent until getData is called.
• previously, the intentResolution type was a JSON data structure - it now has a method - which limits flexibility of implementations.

Would be very helpful to hear more of the thinking behind this.

thanks!

-Nick

[kriswest]

Hi @nkolba, although I raised the PR the decision to use this approach came from the Channels, Feeds & Transactions group. You can find more documentation of the discussion on the issue: #432

TL;DR: the data return might not be immediate, there could be operations to perform or user interactions to wait for (e.g. raising a CreateOrder intent might display and fill out an order ticket, which the user needs to check and confirm before an order Context with an ID is returned). If the data is directly included in the IntentResolution that cannot be returned until the operation completes. Whereas, if you can retrieve a promise for the data, it can be returned immediately, ensuring that the app that raised the intent knows which app is supposed to resolve it.

Regarding the Desktop Agent holding the returned context until getData is called, that's true, although it could be passed from any service implementing the desktop agent to the specific client (in the window that raised the intent) so that the only reference is a resolved promise that can be garbage collected with the IntentResolution when no longer needed. I originally proposed that data become that promise (which I believe helps with your first point, if not your second), but others felt it would make more sense to provide a function to retrieve that promise (I now wonder if that will introduce more implementation complexity based on your question).

[bertrand-s]

I must say I share @nkolba concerns. To be fair I am not clear on the added value of a 2 step process.
As a developer, when I see a function returning a promise I expect the promise to resolve when the function has executed. In this proposal the main promise would be resolved when the intent is dispatched and then we would have to call the 2nd method to get another promise that would resolve when the actual function has been executed. This seems very complicated. What would be the use case of awaiting the first promise and not the 2nd one?
I feel like we have two cases:

• either we want to raise an intent as fire and forget (so we don't even need the first promise)
• or we want to know when the intent is actually processed (even if there is not actual return value) as it allows to chain subsequent actions.
  If we are afraid of performance issues, then why not having an extra argument to the intent method - e.g.:

raiseIntent(intent: string, context: Context, app?: TargetApp, fireAndForget=true): Promise<IntentResolution>;

Besides, promises indeed introduce context, and personally I would rather prefer the context to be handled by the Desktop agent rather than by the apps as it would be easier to fix issues. For instance,

• how to manage backward compatibility? i.e. old apps that expect fire-and-forget intents and that will not return anything?
• what happens if an app crashes while fulfilling an intent?
  In both cases I feel like we would need some kind of timeouts - that could either be handled by the desktop agents or by the FDC3 wrapper library (so on the caller side)

[bertrand-s]

As discussed, couldn't we rename this field getResult() as it would allow to handle potential future use cases where the result is not a data structure (cf. discussions on notifications)

That said, I don't think we need to have a method here (i.e. a Promise would be enough as you suggested). I guess most implementation will keep the promise as a private hidden field that would be returned by getData/getResult method. Indeed we don't want to keep the data in the Agent until the method is called - so the agent is likely to send the result once received and store it in a private field in the client structure to make it available through getData... (so there is no point in this extra complexity..)

[kriswest]

My only concern on using getResult() is that it will require a union return type (Promise<Context | Channel>), typescript is good at those, but other languages are not (instead relying on inheritance patterns - the only common ancestor of these types is object) and although we use Typescript to define the API it should ideally be implementable in other languages.

I would tend to agree on a promise vs. a function returning a promise. Using a function does mean either retaining the data in the desktop agent service or a hidden field, as opposed to it living in the promise itself. However, I have had to demonstrate to a couple of people recently that you can await promises, not just functions returning promises, so I was willing to accept that developers might be more familiar with calling a function.

[kriswest]

Hmm, we also need that union type as the return type for the IntentHandler:

export type IntentHandler = (context: Context) => Promise<Context | PrivateChannel> | void;

We could introduce an explicit union type (as we do for TargetApp). While that doesn't change anything in Typescript, it could be an interface in other languages (such as C#) that is used as a common base for both Context and PrivateChannel to inherit from, allowing either to be returned.

[kriswest]

Updated to getResult() (which will return a union type in a later PR when we add support for returning a channel)

[thorsent]

@nkolba & @bertrand-s (to add to @kriswest) these points are quite valid and were discussed heavily.

The compelling case for a two-stage process was made by several participants, pointing specifically to a common use case in our industry:

• An intent to place a trade should result in (1) a confirmation that the trade was received by the OMS, and (2) a data response that contains a trade identifier (which is often assigned asynchronously). This situation was further clarified in that OMS's often prompt a modal dialog after stage (1) and that, therefore, intent-raising apps should not be frozen during that stage.

The group was swayed, that the additional complexity was warranted given the prevalence of this use case in our industry.

In regards to a function call vs. a plain promise - the initial design did return a plain promise but it was argued that a function that returns a promise would provide a more familiar developer interface. We reworked the approach to compare these approaches and the group agreed that the resulting function call did result in more readable code. This is certainly a stylistic consideration though.

FWIW, an addendum PR is forthcoming that introduces streams into this concept which I think will offer a holistic view for consideration, nomenclature sweeps, etc.

[nkolba]

@thorsent @kriswest @bertrand-s In reply to Terry's comments. I still don't get what we're solving for with getData() .

1. the desktop agent is free to provide end users with any level of feedback they want to that the intent has been routed. For example, in the DesktopAgent chrome extension, the intent dialog closes after a selection is made and the tab of the app processing the intent is brought into focus. ie. in your example, the OMS the intent was routed to could be brought into focus with the modal for the end user to ack.
2. there is nothing requiring blocking on the app raising the intent while it is waiting for the intent to resolve. e.g in the case of data intents, an app could just display a localized loading animation

Has anyone tried implementing these workflows with the data prop? I did some initial implementations last year - as part of the PR I had made and did not find these concerns to be issues.

-Nick

[kriswest]

Hi @bertrand-s

• how to manage backward compatibility? i.e. old apps that expect fire-and-forget intents and that will not return anything?

This proposal is backwards compatible as nothing requires you to call or await the getData() function's promise, ensuring the fire-and-forget intents work exactly the same way as they do now.

• either we want to raise an intent as fire and forget (so we don't even need the first promise)

Raising an intent is inherently an async process, as it involves the delivery of a request/data to another system running in a separate process, potentially mediated by user interaction in a resolver UI.

As it stands currently (and since FDC3 1.0) you don't have to await the raiseIntent promise for the IntentResolution. You only need to do so if you need the info in the IntentResolution (such as the app that resolved the intent) or to know if the resolution was successful (indicated by the promise resolving or rejecting with a ResolveError). This proposal doesn't change that.

Returning data from intent is a two-step process (both steps of which are async operations as described above):

1. resolution/delivery of the intent to an application
2. handling by that application/return of data

If we combine those steps so that they are mediated by a single promise, that will introduce a change in behaviour, as the IntentResolution can't be returned until the handler has finished running, which again might be mediated by a second user interaction (e.g. submitting a form or some other interaction). It would also mean a combined error channel for both resolution and the return of data; if we successfully delivered the intent and context to an app, but it returned an error, we would have to reject the promise of the intent resolution AND would then lack the data structure for expressing which app was responsible for the error. This relates to one of your comments:

what happens if an app crashes while fulfilling an intent?
In both cases I feel like we would need some kind of timeouts - that could either be handled by the desktop agents or by the FDC3 wrapper library (so on the caller side)

The advantage of separating the promises is that, if we trust our Desktop Agent and its resolver UI, we don't need a timeout on the raisedIntent and can find out where it sent our request from the IntentResolution. We can then await the data promise (if we're expecting data), which will reject if an error is thrown. We can implement our own timeout on that wait, without having to have the desktop agent implement it (although that is something we can discuss - the discussion group liked the idea of leaving that to the app raising the intent/expecting data).

@nkolba

the desktop agent is free to provide end users with any level of feedback they want to that the intent has been routed. For example, in the DesktopAgent chrome extension, the intent dialog closes after a selection is made and the tab of the app processing the intent is brought into focus. ie. in your example, the OMS the intent was routed to could be brought into focus with the modal for the end user to ack.

Web containers managing many windows on a multi-screen desktop represent a significantly different form factor to a browser. Focusing a tab is indeed a salient action in a tabbed browser, but far less so in a desktop where it is already visible. I have also recently been reminded by other participants that an Intent resolver UI is not the only possible way to handle intent resolution and that we need to support appropriate feedback through the API. Further, this feedback is intended for the app raisingIntent, and in this case receiving data back, rather than the app resolving the intent. We also considered use-cases where the resolving app has no UI (e.g. a service for retrieving data) and any and all feedback is in the raising application.

there is nothing requiring blocking on the app raising the intent while it is waiting for the intent to resolve. e.g in the case of data intents, an app could just display a localized loading animation.

If you don't use an async construct such as a promise and put the data directly in the IntentResolution then that does require that you block until the handler has run (+any required user interactions) and returned data that you can then put in the resolution. A promise of the data was introduced specifically to prevent the need for that blocking.

If, on the other hand, the objection relates to having to call getData() to retrieve the promise, then I'm actually with you on that. Nothing really requires a function to return it as you can await any expression that evaluates to a promise (i.e. you can just as easily await resolution.dataPromise as await resolution.getData()) however other participants felt using a function would be more familiar to most developers (it could conceivable also be easier to implement in other languages, although I have no specific examples to support that).

[kriswest]

P.S. Thank you both (@bertrand-s and @nkolba) for scrutinizing the PR! I'm happy to make minor changes to it and to clarify design decisions in docs if you think it necessary. Alternative proposals can also be submitted and we can make time to compare them to this one in a further meeting (let me know if you want to do that).

[kriswest]

@greyseer256 I'll merge this one after you've had a chance to a take look, it has the necessary other reviews.

[hughtroeger]

If an error is thrown by the handler function, the promise returned is rejected, or a promise is not returned then the Desktop Agent MUST reject the promise returned by the getResult() function of the IntentResolution.

This sentence occurs several times, I'm still having some difficulty parsing the logic. I'm taking it to mean this:

If an error is thrown by the handler function, the promise returned is rejected, or a promise is not returned, in which case the Desktop Agent MUST reject the promise returned by the getResult() function of the IntentResolution.

is that correct?

worded another way, maybe clearer:
If an error is thrown by the handler function, the promise returned is rejected. If a promise is not returned, the Desktop Agent MUST reject the promise returned by the getResult() function of the IntentResolution.

[kriswest]

@greyseer256 I've clarified the sentence using parentheses wherever it appears, hopefully that's better:

If an error occurs (i.e. an error is thrown by the handler function, the promise returned is rejected, or no promise is returned) then the Desktop Agent MUST reject the promise returned by the getResult() function of the IntentResolution with a string from the DataError enumeration.

and yes, there's a huge amount of duplication on this sentence. Its necessary on two different functions, who's detail is duplicated between the TS sources, docs and spec + same again for the IntentResolution object. One day I hope we'll get to generating the docs from the TS sources which will cut the duplication in half...

[hughtroeger]

Thanks @kriswest, the new wording is much clearer to me.