Fdc3 for web impl get agent refactor #1466
Conversation
Individual timeouts allow for steps that our outside the timeout (identity validation)
… (which needs further work)
ImplementationMetadata is now passed into AbstractMessaging's constructor and getting close to eliminating AbstractWebMessaging. Correcting typing and generics on a number of functions (replacing any and unqualified generics)
…web-impl-getAgent-refactor
- Eliminate AbstractWebMessaging - Messaging is no longer a Connectable (as the connection is provided to it - was previously a no-op) - Handling getInfo via a message exchange rather than passing down through connection flow - Messaging implementations no longer handle implementationMetadata, instead holding the AppIdentifier directly - Renamed HandshakeSupport HeartbeatSupport as its no longer involved in holding implementaiton metadata. - Improving typing and error handling (defensive coding) in get-agent-proxy
…-web-impl-getAgent-refactor
|
@robmoffat @julianna-ciq I think I've got all comments and issues resolved or at least responded, so I only have looking at timeouts (on open/raiseIntent etc.) left to do - and that could be a different PR... Could you check any unresolved comments and see if you're happy or whether you think I need to take it further please. |
… logging tests so it's clearer what they're doing
…impl-getAgent-refactor-rm-patch1
…or-rm-patch1 Refactored FailoverHandler into separate smaller functions, tidied up…
|
|
|
|
|
|
||
| if (response.payload?.channel?.id) { | ||
| //handle successful responses - errors will already have been thrown by exchange above | ||
| if (response.payload.channel) { |
There was a problem hiding this comment.
can't we use your new throwIfUndefined here?
There was a problem hiding this comment.
Have put it in and removed the log message, but the else has to stay so that we return on all branches.
| }; | ||
| handler(event); | ||
| } else { | ||
| console.error('PrivateChannelDisconnectEventListener was called for a different message type!', msg); |
There was a problem hiding this comment.
this line untested - and the message is wrong.
There was a problem hiding this comment.
fixed the message - have not added a test, its just single log line. I do want it to create noise if this starts happening (rather than it happening silently) but ideally it never happens.
| }; | ||
| handler(event); | ||
| } else { | ||
| console.error('PrivateChannelDisconnectEventListener was called for a different message type!', msg); |
There was a problem hiding this comment.
this line untested and the message is wrong
There was a problem hiding this comment.
fixed the message - have not added a test, its just single log line. I do want it to create noise if this starts happening (rather than it happening silently) but ideally it never happens.
|
|
This comment was marked as outdated.
This comment was marked as outdated.
|
I'm seeing a regression running the conformance tests: 
I ran it twice and got the same error both times. The If you want to replicate locally, you'll need to change this line in And you'll need to run the conformance framework, the branch to use is |
That is very likely a result of something I've changed as there was quite a lot of work on the private channel lifecycle events. Thanks for the instructions on replicating the test, I'll give it a go and sort that out. |
This comment was marked as outdated.
This comment was marked as outdated.
Actually scratch that, according to git blame all thats changed on the typing there is formatting with prettier, its been this way for 6 months. I don't understand why it's failing now... As the CI checks don't pick up the build failure that could have been happening for some time I suppose... If its just started I don't get why. Regardless, I can fix it by casting to the target type (HTMLOptionElement) so I've done that... |
|
|
|
|
|
|
|
|
|
|
Describe your change
Significant refactor of the fdc3-get-agent package to align it with the Standard and a number of change fdc3-agent-proxy or fdc3-web-impl that were necessitated by the changes.
Changes:
BasicDesktopAgenttoDesktopAgentProxyto better represent its role.anytypes and type overrides in many places - there are others still to addressgetAgentimplementation considering its own window to be a possible parent and to set up messaging with it 0t this is now preventedRelated Issue
Addresses a number of issues in addition to working to improve the implementation generally:
resolves #1429
resolves #1430
resolves #1433
resolves #1445
resolves #1468
Contributor License Agreement
Review Checklist
DesktopAgent,Channel,PrivateChannel,Listener,Bridging)?JSDoc comments on interfaces and types should be matched to the main documentation in /docs
Conformance test definitions should cover all required aspects of an FDC3 Desktop Agent implementation, which are usually marked with a MUST keyword, and optional features (SHOULD or MAY) where the format of those features is defined
The Web Connection protocol and Desktop Agent Communication Protocol schemas must be able to support all necessary aspects of the Desktop Agent API, while Bridging must support those aspects necessary for Desktop Agents to communicate with each other
npm run build) run and the results checked in?Generated code will be found at
/src/api/BrowserTypes.tsand/or/src/bridging/BridgingTypes.tsBaseContextschema applied viaallOf(as it is in existing types)?titleanddescriptionprovided for all properties defined in the schema?npm run build) run and the results checked in?Generated code will be found at
/src/context/ContextTypes.ts