-
Notifications
You must be signed in to change notification settings - Fork 130
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
broadcastEvent on no channel #1310
Comments
Can I have some more details on how this is supposed to work? I assume that the broadcast message is only received by the newly opened app if it calls addContextListener on boot? Also how is it supposed to work with the channels as Rob is asking above. Thanks |
I never thought of this workflow myself for open(). I always assumed the workflow is open(no context), app starts, performs the addIntentListener and gets the pending context based on the intent. |
@Roaders @robmoffat there was some discussion of this over email while I was on vacation - I forget who was on that email. @robmoffat @Roaders Essentially, if an app is opened via This is the same as what happens with intents, theres just no intent passed along and we use a I will happily say I don't think much of this part of the FDC3 API design as there is some unnecessary complexity in the waiting around for the listener to be added and I dislike the reuse of the listener for a slightly different purpose (it has no way to tell if it came off a channel or via open) but its been there a long time. In the protocol, we'll need to document that the app should be opened, the Desktop Agent should wait appropriate Listener(s) to be added and then send a specific broadcastEvent message only to that app to deliver the context. No special code in the proxy should be needed as Please note that there are some timeouts for adding the listeners specified - this was my attempt to put some bounds on the waiting, see: In the end what we added was:
https://fdc3.finos.org/docs/api/spec#desktop-agent-api-standard-compliance
https://fdc3.finos.org/docs/context/spec#context-data-standard-compliance
https://fdc3.finos.org/docs/intents/spec#intents-standard-compliance Finally, please note this error that can be retruned from enum OpenError {
...
/** Returned if the specified application launches but fails to add a context
* listener in order to receive the context passed to the `fdc3.open` call.
*/
AppTimeout = 'AppTimeout',
} Its existence implies that you don't resolve |
Thanks for the explanation.
No special code other than having to act on a message with no Basically I think the way we need to write it is to do no filtering of messages on the agents and no checking that the raiseIntent message for example is intended for that particular agent. If we get a message we act on it I guess. Our main implementation is for the correct "smart" message routing so that all messages are only sent to the intended agent. We are allowing the messaging to be swappable though so some users might implement a stupid system that does not route messages. By the sounds of it we need to remove that requirement. |
Correct - and to be clear that applies to the API standard, not just this protocol supporting it. It's rarely a good idea to send everything everywhere - its harder to secure, it creates additional traffic and processing to do in threads/apps that wouldn't need to do any otherwise, affecting performance and (your ability to implement) security. Returning to terminology for a second, there is conceptually a single 'agent' with an 'API endpoint' for talking to it in each application, rather than an 'agent' in each. There is this line in the API overview about a Desktop Agent:
Note applications is plural and routes messages between them (rather than just filtering whats on a bus). I hope that helps provide some background. |
@robmoffat and @Roaders see a42798c - this should fix the issue with channelId on broadcastEvent. channelId MUST be set, but set it THis will need documenting in the communication protocol, hoping to get onto that this week, once I've got through your backlog of questions/comments. |
Closing on the assumption that the fix is good, reopen if not! |
Minor Issue
When an app (A) is opened with context and adds it's context listener it probably won't have a user channel set.
The desktop agent is required to send a
broadcastEvent
to the app with the context, but currently requires a channelId to be set in the schema.Is this right or not?
The text was updated successfully, but these errors were encountered: