866 app add listener timeouts #987
Conversation
kriswest
added
enhancement
✅ Deploy Preview for fdc3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
@kriswest . Don't agree with the MUST here. Especially since the language seems to assume there is a single flow in which apps add context and intent listeners (i.e. immediately on startup). |
There was a problem hiding this comment.
90 second requirement seems overly proscriptive and assuming a specific pattern and flow for apps that may not be the case. What is the reason for requiring this?
There was a problem hiding this comment.
The flow is prescribed by fdc3.open and fdc3.raiseIntent and the passing of context to an app on launch and these requirements specifically linked to that case.
As per the issue, the lack of any advice or bound on the timeouts means both app developers and desktop agent developers are guessing at values. That in turn makes conformance testing harder as we don't know how long you've got to wait before deciding the DA has failed to notice the failure to add the listener and to subsequently return the relevant error.
The point was to add a range and a recommendation to help guide implementors, nothing more.
I'm told no one that has conformance tested has used a timeout longer than 70 seconds, so I went higher at 90. I further posted to the FDC3 maintainers to ask if anyone has seen higher or wanted to feedback on it.
There was a problem hiding this comment.
@kriswest guidance and recommendations are good. But what is gained from the MUST? The conformance tests are a closed loop, so the only reason for a timeout of intent resolution in the conformance tests is if something else is broken. What's the use case for this being a MUST and are there other use cases that we might end up blocking?
There was a problem hiding this comment.
Unless we know what the maximum time is, the conformance test can't finish.
In the case 2.0-RaiseIntentFailTargetedAppResolve3 where the desktop agent is waiting for a newly-opened app to respond to an intent (and it doesn't) then the desktop agent should return ResolveError.IntentDeliveryFailed.
However, if the test calls it (and fails) at 90 seconds, but the desktop agent has a timeout of 95 seconds, then the test will fail erroneously - the DA is performing according to specification but is doing it beyond the wait time of the test.
There was a problem hiding this comment.
@nkolba as discussed in the maintainers channel the reason for there being a timeout in the conformance test is that Desktop Agents are already required to return a particular error from ResolveError or OpenError on failure to deliver an intent/context, hence, at some point it has to decide that the DA is not going to return that error and is just hanging. Without a known timeout or at least some bound on the timeout there's no way to know whether that is the case if it doesn't return. The same applies to an application awaiting an open or raiseIntent call - without there being a bound on the timeout's used, they might need to implement their own.
An alternative is to just make it a SHOULD. However, that's vague and hand-wavy - an app developer or adopter can't rely on it. Whereas, a stronger requirement should encourage developers to find alternative solutions in cases where their app start-up is longer than the proposed bound.
An alternative way to soften the requirement would be to put the bound on the default timeout and add a MAY allowing the DA to be configured to use a longer timeout for particular apps. I believe you were not keen on adding such to the appD record in standard place, but they could do so within a hostManifest easily enough. Perhaps that is the best solution as it both encourages better behavior by default, but also allows for cases where more than the bounded timeout is needed.
There was a problem hiding this comment.
@robmoffat @kriswest Couldn't the conformance tests just set their own timeout? Is there any downside? The tests are a known quantity and there is no reason for them to get anywhere even close to 90 seconds unless the test was broken.
There was a problem hiding this comment.
They do set their own timeout, but what to set it to is unknown. The point of those tests is to confirm the behaviour of the desktop agent (response to raiseIntent or open) when the app doesn't behave correctly/is broken.
This isn't just about making it easier to test standard behaviour, rather it's to make what the standard behaviour is (for both apps and Desktop Agents) clearer.
|
The lack of a defined timeout was independently raised again by a team from a Desktop Agent vendor reviewing the conformance testing framework. In that case, they are counting the timeout from when the intent is raised, rather than from when the app is spawned, hence any time a resolver UI or similar is open is included. Counting from when the intent is raised has the advantage that the app raising the intent can know how long it might be open. However, it makes the timeout available to the launched app variable as time is likely to have been taken up with a user interacting with a resolver UI. Personally, I think it more useful to advise the app being launched as to how quickly they should be adding their listener - particularly as the app initiating the action ( |
|
Updated the draft to clarify that the timeouts only apply to intents and contexts that should be receivable on launch via |
|
Please see my comment on the related issue. I think it is a mistake to push demands about timeout behaviour from the conformance layer into the standard. I think the expected timeouts can live in and be dictated by the conformance tests - if you don't fit into their expected timeouts, you can't get certified. But because we are dealing with real users and UIs, the edges are too hard in terms of imposing conditions inside the standard itself for this, and that is why it was never done. |
Thats not what is intended, rather it was conversations between vendors during meetings looking at the tests that flagged a vague part of the standard that might be more helpful (to multiple stakeholders) if it was more prescriptive.
When a Standard ducks out of addressing hard issues, its just pushing those issues down to its users/implementors in my opinion. Where we're dealing with multiple stakeholders (a couple of app vendors, a user, a DA vendor) that can get quite complex when they vary (e.g. swapping out different apps, different DAs), which is why standardizing behaviour is useful. This is a relatively small issue, and clearly the upper bound is more controversial (despite the allowance for config to vary/extend it that was added?). A you less uncomfortable with shorter value for at least a minimum bound? E.g. DAs MUST alow at least 10 seconds? Or are you against any form of clarification on how quickly apps should add listeners for use cases where they are handed context/intents on startup? |
…xt and Intent specs
Clarifies that the timeouts only apply to intents and contexts that should be received on launch of the application and allows for them to be extended for individual applications via configuration.
… minimum and recommedation to do it within that minimum
kriswest
force-pushed
the
866-app-add-listener-timeouts
branch
from
a53ee14
to
25d67f5
Compare
|
In the interest of agreeing some form of improvement, I've adjusted the PR to only include:
Hopefully this version is acceptable @rikoe and @nkolba - if you'd like further change please suggest wording if at all possible. |
|
@kriswest these changes look better to me, now that the MUST is only on the minimum for a timeout. Would still prefer no MUST here (an implementor could feasibly have no timeouts and just allow the end user to cancel the operation if the launching app never responded), but this seems OK. |
This was discussed at the maintainers meeting: The existence of a minimum timeout doesn't preclude having a (near) infinite timeout so it continues until the user somehow cancels the launch operation. The timeout applies after resolution on app launch, so this is different from the user closing a resolver UI (which should result in |
| * If the channel already contains context that would be passed to context listeners assed via `fdc3.addContextListener` then those listeners will be called immediately with that context. | ||
| * If the channel already contains context that would be passed to context listeners added via `fdc3.addContextListener` then those listeners will be called immediately with that context. |
Co-authored-by: Hugh Troeger <htroeger@factset.com>
resolves #866
Adds details of timeouts after launch within which applications SHOULD (30 seconds) and MUST (90 seconds) add their context or intent listeners via the API, and details of the same timeouts being allowed for by Desktop Agents (MUST 30 seconds, SHOULD 90 seconds).