Skip to content

Commit

Permalink
refactor(devtools-core): Make message handlers async (#15792)
Browse files Browse the repository at this point in the history
Updates our message handler infra to make handlers async. The current
motivation for this is to allow the data visualization code to run to
completion before we report whether or not the message was handled.

### Breaking Change

This PR changes the message handler interface, which is exported and
used by the view.
  • Loading branch information
Josmithr authored Jun 1, 2023
1 parent 6433cb2 commit 26eb70e
Show file tree
Hide file tree
Showing 13 changed files with 45 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ export interface IMessageRelayEvents<TMessage extends ISourcedDevtoolsMessage =

// @internal
export interface InboundHandlers {
[type: string]: (message: ISourcedDevtoolsMessage) => boolean;
[type: string]: (message: ISourcedDevtoolsMessage) => Promise<boolean>;
}

// @public
Expand Down
26 changes: 12 additions & 14 deletions packages/tools/devtools/devtools-core/src/ContainerDevtools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,70 +272,68 @@ export class ContainerDevtools implements IContainerDevtools, HasContainerKey {
* Handlers for inbound messages related to the debugger.
*/
private readonly inboundMessageHandlers: InboundHandlers = {
[GetContainerDevtoolsFeatures.MessageType]: (untypedMessage) => {
[GetContainerDevtoolsFeatures.MessageType]: async (untypedMessage) => {
const message = untypedMessage as GetContainerDevtoolsFeatures.Message;
if (message.data.containerKey === this.containerKey) {
this.postSupportedFeatures();
return true;
}
return false;
},
[GetContainerState.MessageType]: (untypedMessage) => {
[GetContainerState.MessageType]: async (untypedMessage) => {
const message = untypedMessage as GetContainerState.Message;
if (message.data.containerKey === this.containerKey) {
this.postContainerStateChange();
return true;
}
return false;
},
[ConnectContainer.MessageType]: (untypedMessage) => {
[ConnectContainer.MessageType]: async (untypedMessage) => {
const message = untypedMessage as ConnectContainer.Message;
if (message.data.containerKey === this.containerKey) {
this.container.connect();
return true;
}
return false;
},
[DisconnectContainer.MessageType]: (untypedMessage) => {
[DisconnectContainer.MessageType]: async (untypedMessage) => {
const message = untypedMessage as DisconnectContainer.Message;
if (message.data.containerKey === this.containerKey) {
this.container.disconnect(/* TODO: Specify debugger reason here once it is supported */);
return true;
}
return false;
},
[CloseContainer.MessageType]: (untypedMessage) => {
[CloseContainer.MessageType]: async (untypedMessage) => {
const message = untypedMessage as CloseContainer.Message;
if (message.data.containerKey === this.containerKey) {
this.container.close(/* TODO: Specify debugger reason here once it is supported */);
return true;
}
return false;
},
[GetAudienceSummary.MessageType]: (untypedMessage) => {
[GetAudienceSummary.MessageType]: async (untypedMessage) => {
const message = untypedMessage as GetAudienceSummary.Message;
if (message.data.containerKey === this.containerKey) {
this.postAudienceStateChange();
return true;
}
return false;
},
[GetRootDataVisualizations.MessageType]: (untypedMessage) => {
[GetRootDataVisualizations.MessageType]: async (untypedMessage) => {
const message = untypedMessage as GetRootDataVisualizations.Message;
if (message.data.containerKey === this.containerKey) {
this.getRootDataVisualizations().then((visualizations) => {
this.postRootDataVisualizations(visualizations);
}, console.error);
const visualizations = await this.getRootDataVisualizations();
this.postRootDataVisualizations(visualizations);
return true;
}
return false;
},
[GetDataVisualization.MessageType]: (untypedMessage) => {
[GetDataVisualization.MessageType]: async (untypedMessage) => {
const message = untypedMessage as GetDataVisualization.Message;
if (message.data.containerKey === this.containerKey) {
this.getDataVisualization(message.data.fluidObjectId).then((visualization) => {
this.postDataVisualization(message.data.fluidObjectId, visualization);
}, console.error);
const visualization = await this.getDataVisualization(message.data.fluidObjectId);
this.postDataVisualization(message.data.fluidObjectId, visualization);
return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class DevtoolsLogger extends TelemetryLogger {
* Handlers for inbound messages related to the logger.
*/
private readonly inboundMessageHandlers: InboundHandlers = {
[GetTelemetryHistory.MessageType]: (untypedMessage) => {
[GetTelemetryHistory.MessageType]: async (untypedMessage) => {
this.postLogHistory();
return true;
},
Expand Down
4 changes: 2 additions & 2 deletions packages/tools/devtools/devtools-core/src/FluidDevtools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,11 @@ export class FluidDevtools implements IFluidDevtools {
* Handlers for inbound messages specific to FluidDevTools.
*/
private readonly inboundMessageHandlers: InboundHandlers = {
[GetDevtoolsFeatures.MessageType]: () => {
[GetDevtoolsFeatures.MessageType]: async () => {
this.postSupportedFeatures();
return true;
},
[GetContainerList.MessageType]: () => {
[GetContainerList.MessageType]: async () => {
this.postContainerList();
return true;
},
Expand Down
28 changes: 19 additions & 9 deletions packages/tools/devtools/devtools-core/src/messaging/Utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ export function postMessagesToWindow<TMessage extends IDevtoolsMessage>(
export interface InboundHandlers {
/**
* Mapping from {@link IDevtoolsMessage."type"}s to a handler callback for that message type.
* @returns Whether or not the message was actually handled.
*
* @returns A promise that resolves to a flag indicating if the message was handled (`true`) or not (`false`).
*/
[type: string]: (message: ISourcedDevtoolsMessage) => boolean;
[type: string]: (message: ISourcedDevtoolsMessage) => Promise<boolean>;
}

/**
Expand Down Expand Up @@ -113,14 +114,23 @@ export function handleIncomingMessage(
return;
}

const handled = handlers[message.type](message);
const loggingPreamble =
loggingOptions?.context === undefined ? "" : `${loggingOptions.context}: `;

// Only log if the message was actually handled by the recipient.
if (handled && loggingOptions !== undefined) {
const loggingPreamble =
loggingOptions?.context === undefined ? "" : `${loggingOptions.context}: `;
console.debug(`${loggingPreamble} message handled:`, message);
}
handlers[message.type](message).then(
(wasMessageHandled) => {
// Only log if the message was actually handled by the recipient.
if (wasMessageHandled && loggingOptions !== undefined) {
console.debug(`${loggingPreamble} Message handled.`, message);
}
},
(error) => {
console.error(
`${loggingPreamble} Message could not be handled due to an error:`,
error,
);
},
);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/tools/devtools/devtools-view/src/DevtoolsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export function DevtoolsView(): React.ReactElement {
* Handlers for inbound messages related to the registry.
*/
const inboundMessageHandlers: InboundHandlers = {
[DevtoolsFeatures.MessageType]: (untypedMessage) => {
[DevtoolsFeatures.MessageType]: async (untypedMessage) => {
const message = untypedMessage as DevtoolsFeatures.Message;
setSupportedFeatures(message.data.features);
return true;
Expand Down Expand Up @@ -265,7 +265,7 @@ function _DevtoolsView(props: _DevtoolsViewProps): React.ReactElement {
* Handlers for inbound messages related to the registry.
*/
const inboundMessageHandlers: InboundHandlers = {
[ContainerList.MessageType]: (untypedMessage) => {
[ContainerList.MessageType]: async (untypedMessage) => {
const message = untypedMessage as ContainerList.Message;
setContainers(message.data.containers);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function AudienceView(props: AudienceViewProps): React.ReactElement {
* Handlers for inbound messages related to Audience
*/
const inboundMessageHandlers: InboundHandlers = {
[AudienceSummary.MessageType]: (untypedMessage) => {
[AudienceSummary.MessageType]: async (untypedMessage) => {
const message = untypedMessage as AudienceSummary.Message;

setAudienceData(message.data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export function ContainerDevtoolsView(props: ContainerDevtoolsViewProps): React.
* Handlers for inbound messages related to the registry.
*/
const inboundMessageHandlers: InboundHandlers = {
[ContainerDevtoolsFeatures.MessageType]: (untypedMessage) => {
[ContainerDevtoolsFeatures.MessageType]: async (untypedMessage) => {
const message = untypedMessage as ContainerDevtoolsFeatures.Message;
if (message.data.containerKey === containerKey) {
setSupportedFeatures(message.data.features);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function ContainerHistoryView(props: ContainerHistoryProps): React.ReactE
* Handlers for inbound messages related to the registry.
*/
const inboundMessageHandlers: InboundHandlers = {
[ContainerStateHistory.MessageType]: (untypedMessage) => {
[ContainerStateHistory.MessageType]: async (untypedMessage) => {
const message = untypedMessage as ContainerStateHistory.Message;
if (message.data.containerKey === containerKey) {
setContainerHistory(message.data.history);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export function ContainerSummaryView(props: ContainerSummaryViewProps): React.Re
* Handlers for inbound messages related to the registry.
*/
const inboundMessageHandlers: InboundHandlers = {
[ContainerStateChange.MessageType]: (untypedMessage) => {
[ContainerStateChange.MessageType]: async (untypedMessage) => {
const message = untypedMessage as ContainerStateChange.Message;
if (message.data.containerKey === containerKey) {
setContainerState(message.data.containerState);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function DataObjectsView(props: DataObjectsViewProps): React.ReactElement

React.useEffect(() => {
const inboundMessageHandlers: InboundHandlers = {
[RootDataVisualizations.MessageType]: (untypedMessage) => {
[RootDataVisualizations.MessageType]: async (untypedMessage) => {
const message = untypedMessage as RootDataVisualizations.Message;

if (message.data.containerKey === containerKey) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ export function TelemetryView(): React.ReactElement {
* Handlers for inbound messages related to telemetry.
*/
const inboundMessageHandlers: InboundHandlers = {
[TelemetryEvent.MessageType]: (untypedMessage) => {
[TelemetryEvent.MessageType]: async (untypedMessage) => {
const message = untypedMessage as TelemetryEvent.Message;
setBufferedEvents((currentBuffer) => [
message.data.event,
...(currentBuffer ?? []),
]);
return true;
},
[TelemetryHistory.MessageType]: (untypedMessage) => {
[TelemetryHistory.MessageType]: async (untypedMessage) => {
const message = untypedMessage as TelemetryHistory.Message;
setTelemetryEvents(message.data.contents);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function FluidHandleView(props: FluidHandleViewProps): React.ReactElement
* Handlers for inbound message related to Data View.
*/
const inboundMessageHandlers: InboundHandlers = {
[DataVisualization.MessageType]: (untypedMessage) => {
[DataVisualization.MessageType]: async (untypedMessage) => {
const message = untypedMessage as DataVisualization.Message;
if (
message.data.containerKey === containerKey &&
Expand Down

0 comments on commit 26eb70e

Please sign in to comment.