Skip to content
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

handleMessage default behaviour leads to superfluous errors #53

Open
jryans opened this issue Jan 31, 2022 · 0 comments
Open

handleMessage default behaviour leads to superfluous errors #53

jryans opened this issue Jan 31, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@jryans
Copy link
Contributor

jryans commented Jan 31, 2022

On each side of the widget transport, there are handleMessage functions such as:

    private handleMessage(ev: CustomEvent<IWidgetApiRequest>) {
        const actionEv = new CustomEvent(`action:${ev.detail.action}`, {
            detail: ev.detail,
            cancelable: true,
        });
        this.emit(`action:${ev.detail.action}`, actionEv);
        if (!actionEv.defaultPrevented) {
            switch (ev.detail.action) {
                case WidgetApiToWidgetAction.SupportedApiVersions:
                    return this.replyVersions(<ISupportedVersionsActionRequest>ev.detail);
                case WidgetApiToWidgetAction.Capabilities:
                    return this.handleCapabilities(<ICapabilitiesActionRequest>ev.detail);
                case WidgetApiToWidgetAction.UpdateVisibility:
                    return this.transport.reply(ev.detail, <IWidgetApiRequestEmptyData>{}); // ack to avoid error spam
                case WidgetApiToWidgetAction.NotifyCapabilities:
                    return this.transport.reply(ev.detail, <IWidgetApiRequestEmptyData>{}); // ack to avoid error spam
                default:
                    return this.transport.reply(ev.detail, <IWidgetApiErrorResponseData>{
                        error: {
                            message: "Unknown or unsupported action: " + ev.detail.action,
                        },
                    });
            }
        }
    }

At the moment, the library seems setup (based on the guide on the repo home page) to require widget developers to call ev.preventDefault() on anything they listen to and craft their own reply, even when it's effectively a one way notification and an empty reply is just fine.

As shown above a few random notification-like actions (UpdateVisibility, NotifyCapabilities) are auto-replied, while all others (e.g. SendEvent) default to sending back an error, even though the data was emitted and probably processed by the widget anyway.

I would suggest flipping around the default behaviour to sending empty data and dropping the error path. After all, the data has been emitted already, and (most likely) processed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant