Skip to content

fix(core): make handleUnsubscribeEvent and handleUnobserveProperty async #1480

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.

Sign up for GitHub

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

Open
Pranav-0440 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(core): make handleUnsubscribeEvent and handleUnobserveProperty async #1480

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa2e658
fix(core): make handleUnsubscribeEvent and handleUnobserveProperty async
Pranav-0440 Feb 12, 2026
bc72d1d
fix(bindings): handle async unsubscribe/unobserve without floating pr…
Pranav-0440 Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/core/src/exposed-thing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -572,11 +572,11 @@ export default class ExposedThing extends TD.Thing implements WoT.ExposedThing {
*
* @experimental
*/
public handleUnsubscribeEvent(
public async handleUnsubscribeEvent(
name: string,
listener: ContentListener,
options: WoT.InteractionOptions & { formIndex: number }
): void {
): Promise<void> {
Comment on lines +575 to +579
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change. The method signature is being changed from returning void to returning Promise<void>, but none of the callers in the binding implementations are being updated to await this promise. This will result in floating promises and broken error handling.

All callers need to be updated to await this method:

  • packages/binding-coap/src/coap-server.ts:889
  • packages/binding-http/src/routes/event.ts:89, 102, 104
  • packages/binding-mqtt/src/mqtt-broker-server.ts:198
  • packages/binding-websockets/src/ws-server.ts:296
  • packages/core/test/server-test.ts:967

Many of these calls are in event handlers (res.on("close"), res.on("finish"), ws.on("close"), setTimeout) where the returned promise may not be properly handled. Consider whether this change should be made atomically with updates to all callers, or if error handling needs to be added to gracefully handle floating promises in these contexts.

Copilot uses AI. Check for mistakes.
if (this.events[name] != null) {
Helpers.validateInteractionOptions(this, this.events[name], options);

Expand All @@ -595,7 +595,7 @@ export default class ExposedThing extends TD.Thing implements WoT.ExposedThing {
}
const unsubscribe = this.#eventHandlers.get(name)?.unsubscribe;
if (unsubscribe) {
unsubscribe(options);
await unsubscribe(options);
}
debug(`ExposedThing '${this.title}' unsubscribes from event '${name}'`);
} else {
Expand Down Expand Up @@ -639,11 +639,11 @@ export default class ExposedThing extends TD.Thing implements WoT.ExposedThing {
}
}

Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @experimental JSDoc tag. The corresponding methods handleSubscribeEvent, handleUnsubscribeEvent, and handleObserveProperty all have the @experimental tag. This method should be consistently marked as experimental as well.

Suggested change
/**
*
* @experimental
*/

Copilot uses AI. Check for mistakes.
public handleUnobserveProperty(
public async handleUnobserveProperty(
name: string,
listener: ContentListener,
options: WoT.InteractionOptions & { formIndex: number }
): void {
): Promise<void> {
Comment on lines +642 to +646
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change. The method signature is being changed from returning void to returning Promise<void>, but none of the callers in the binding implementations are being updated to await this promise. This will result in floating promises and broken error handling.

All callers need to be updated to await this method:

  • packages/binding-coap/src/coap-server.ts:701, 704
  • packages/binding-http/src/routes/property-observe.ts:91, 107, 109
  • packages/binding-websockets/src/ws-server.ts:236

Many of these calls are in event handlers (res.on("finish"), setTimeout) where the returned promise may not be properly handled. Consider whether this change should be made atomically with updates to all callers, or if error handling needs to be added to gracefully handle floating promises in these contexts.

Copilot uses AI. Check for mistakes.
if (this.properties[name] != null) {
Helpers.validateInteractionOptions(this, this.properties[name], options);
const formIndex = ProtocolHelpers.getFormIndexForOperation(
Expand All @@ -663,7 +663,7 @@ export default class ExposedThing extends TD.Thing implements WoT.ExposedThing {

const unobserveHandler = this.#propertyHandlers.get(name)?.unobserveHandler;
if (unobserveHandler) {
unobserveHandler(options);
await unobserveHandler(options);
}
} else {
throw new Error(`ExposedThing '${this.title}', no property found for '${name}'`);
Expand Down
Loading