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

Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions packages/binding-coap/src/coap-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,10 +698,10 @@ export default class CoapServer implements ProtocolServer {

res.on("finish", (err: Error) => {
error(`CoapServer on port ${this.port} failed on observe with: ${err.message}`);
thing.handleUnobserveProperty(affordanceKey, listener, interactionOptions);
void thing.handleUnobserveProperty(affordanceKey, listener, interactionOptions);
Copy link
Copy Markdown
Member

@danielpeintner danielpeintner Feb 18, 2026

Choose a reason for hiding this comment

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

Shouldn't we handle error cases?

});

setTimeout(() => thing.handleUnobserveProperty(affordanceKey, listener, interactionOptions), 60 * 60 * 1000);
setTimeout(() => void thing.handleUnobserveProperty(affordanceKey, listener, interactionOptions), 60 * 60 * 1000);
}

private createContentListener(
Expand Down Expand Up @@ -886,7 +886,7 @@ export default class CoapServer implements ProtocolServer {
req.rsinfo.address
)}:${req.rsinfo.port}`
);
thing.handleUnsubscribeEvent(affordanceKey, listener, interactionOptions);
void thing.handleUnsubscribeEvent(affordanceKey, listener, interactionOptions);
});
} else if (observe > 0) {
debug(
Expand Down
6 changes: 3 additions & 3 deletions packages/binding-http/src/routes/event.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export default async function eventRoute(
} catch (err) {
// Safe cast to NodeJS.ErrnoException we are checking if it is equal to ERR_HTTP_HEADERS_SENT
if ((err as NodeJS.ErrnoException)?.code === "ERR_HTTP_HEADERS_SENT") {
thing.handleUnsubscribeEvent(eventName, listener, options);
void thing.handleUnsubscribeEvent(eventName, listener, options);
return;
}
const message = err instanceof Error ? err.message : JSON.stringify(err);
Expand All @@ -99,9 +99,9 @@ export default async function eventRoute(
await thing.handleSubscribeEvent(eventName, listener, options);
res.on("close", () => {
debug(`HttpServer on port ${this.getPort()} closed Event connection`);
thing.handleUnsubscribeEvent(eventName, listener, options);
void thing.handleUnsubscribeEvent(eventName, listener, options);
});
res.setTimeout(60 * 60 * 1000, () => thing.handleUnsubscribeEvent(eventName, listener, options));
res.setTimeout(60 * 60 * 1000, () => void thing.handleUnsubscribeEvent(eventName, listener, options));
} else if (req.method === "HEAD") {
// HEAD support for long polling subscription
res.writeHead(202);
Expand Down
6 changes: 3 additions & 3 deletions packages/binding-http/src/routes/property-observe.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export default async function propertyObserveRoute(
} catch (err) {
// Safe cast to NodeJS.ErrnoException we are checking if it is equal to ERR_HTTP_HEADERS_SENT
if ((err as NodeJS.ErrnoException)?.code === "ERR_HTTP_HEADERS_SENT") {
thing.handleUnobserveProperty(propertyName, listener, options);
void thing.handleUnobserveProperty(propertyName, listener, options);
return;
}
const message = err instanceof Error ? err.message : JSON.stringify(err);
Expand All @@ -104,9 +104,9 @@ export default async function propertyObserveRoute(
await thing.handleObserveProperty(_params.property, listener, options);
res.on("finish", () => {
debug(`HttpServer on port ${this.getPort()} closed connection`);
thing.handleUnobserveProperty(propertyName, listener, options);
void thing.handleUnobserveProperty(propertyName, listener, options);
});
res.setTimeout(60 * 60 * 1000, () => thing.handleUnobserveProperty(propertyName, listener, options));
res.setTimeout(60 * 60 * 1000, () => void thing.handleUnobserveProperty(propertyName, listener, options));
} else if (req.method === "HEAD") {
// HEAD support for long polling subscription
// TODO: set the Content-Type header to the type of the property
Expand Down
2 changes: 1 addition & 1 deletion packages/binding-mqtt/src/mqtt-broker-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export default class MqttBrokerServer implements ProtocolServer {

if (content == null) {
warn(`MqttBrokerServer on port ${this.getPort()} cannot process data for Event ${eventName}`);
thing.handleUnsubscribeEvent(eventName, eventListener, { formIndex: event.forms.length - 1 });
void thing.handleUnsubscribeEvent(eventName, eventListener, { formIndex: event.forms.length - 1 });
return;
}
debug(`MqttBrokerServer at ${this.brokerURI} publishing to Event topic '${eventName}' `);
Expand Down
4 changes: 2 additions & 2 deletions packages/binding-websockets/src/ws-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ export default class WebSocketServer implements ProtocolServer {

ws.on("close", () => {
for (let formIndex = 0; formIndex < thing.properties[propertyName].forms.length; formIndex++) {
thing.handleUnobserveProperty(propertyName, observeListener, { formIndex });
void thing.handleUnobserveProperty(propertyName, observeListener, { formIndex });
}
debug(
`WebSocketServer on port ${this.getPort()} closed connection for '${path}' from ${Helpers.toUriLiteral(
Expand Down Expand Up @@ -293,7 +293,7 @@ export default class WebSocketServer implements ProtocolServer {

ws.on("close", () => {
for (let formIndex = 0; formIndex < event.forms.length; formIndex++) {
thing.handleUnsubscribeEvent(eventName, eventListener, { formIndex });
void thing.handleUnsubscribeEvent(eventName, eventListener, { formIndex });
}
debug(
`WebSocketServer on port ${this.getPort()} closed connection for '${path}' from ${Helpers.toUriLiteral(
Expand Down
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