fix service race condition

src/core/Service.js

@@ -94,7 +94,9 @@ export default class Service extends EventEmitter {
    */
   advertise(callback) {
     if (this.isAdvertised) {
-      throw new Error('Cannot advertise the same Service twice!');
+      // If already advertised, unadvertise first then re-advertise with new callback
+      // This prevents throwing an error and allows smooth re-advertising
+      this.unadvertise();
     }
 
     // Store the new callback for removal during un-advertisement
@@ -127,17 +129,26 @@ export default class Service extends EventEmitter {
 
   unadvertise() {
     if (!this.isAdvertised) {
-      throw new Error(`Tried to un-advertise service ${this.name}, but it was not advertised!`);
+      // Silently return if not advertised instead of throwing an error
+      // This prevents race conditions where multiple unadvertise calls happen
+      return;
     }
-    this.ros.callOnConnection({
-      op: 'unadvertise_service',
-      service: this.name
-    });
-    // Remove the registered callback
+
+    // Remove the registered callback first to stop processing new requests
     if (this._serviceCallback) {
       this.ros.off(this.name, this._serviceCallback);
+      this._serviceCallback = null;
     }
+
+    // Mark as not advertised before sending the message
+    // This ensures that any new advertise calls won't be blocked
     this.isAdvertised = false;
+
+    // Send the unadvertise message to the server
+    this.ros.callOnConnection({
+      op: 'unadvertise_service',
+      service: this.name
+    });
   }
 
   /**
@@ -146,8 +157,11 @@ export default class Service extends EventEmitter {
    */
   advertiseAsync(callback) {
     if (this.isAdvertised) {
-      throw new Error('Cannot advertise the same Service twice!');
+      // If already advertised, unadvertise first then re-advertise with new callback
+      // This prevents throwing an error and allows smooth re-advertising
+      this.unadvertise();
     }
+
     this._serviceCallback = async (rosbridgeRequest) => {
       /** @type {{op: string, service: string, values?: TResponse, result: boolean, id?: string}} */
       let rosbridgeResponse = {

test/service.test.js

@@ -52,4 +52,97 @@ describe('Service', () => {
     server.unadvertise();
     expect(ros.listenerCount(server.name)).toEqual(0);
   })
+
+  it('Handles re-advertisement gracefully without throwing errors', async () => {
+    const server = new Service({
+      ros,
+      serviceType: 'std_srvs/Trigger',
+      name: '/test_readvertise'
+    });
+
+    // First advertisement
+    server.advertise((_request, response) => {
+      response.success = true;
+      response.message = 'first';
+      return true;
+    });
+
+    expect(server.isAdvertised).toBe(true);
+    expect(ros.listenerCount(server.name)).toEqual(1);
+
+    // Re-advertise with different callback - should not throw
+    expect(() => {
+      server.advertise((_request, response) => {
+        response.success = true;
+        response.message = 'second';
+        return true;
+      });
+    }).not.toThrow();
+
+    expect(server.isAdvertised).toBe(true);
+    expect(ros.listenerCount(server.name)).toEqual(1);
+
+    server.unadvertise();
+    expect(server.isAdvertised).toBe(false);
+    expect(ros.listenerCount(server.name)).toEqual(0);
+  })
+
+  it('Handles multiple unadvertise calls gracefully', () => {
+    const server = new Service({
+      ros,
+      serviceType: 'std_srvs/Trigger',
+      name: '/test_multiple_unadvertise'
+    });
+
+    server.advertise((_request, response) => {
+      response.success = true;
+      return true;
+    });
+
+    expect(server.isAdvertised).toBe(true);
+
+    // First unadvertise
+    server.unadvertise();
+    expect(server.isAdvertised).toBe(false);
+
+    // Second unadvertise - should not throw
+    expect(() => {
+      server.unadvertise();
+    }).not.toThrow();
+
+    expect(server.isAdvertised).toBe(false);
+  })
+
+  it('Handles re-advertisement with advertiseAsync gracefully', async () => {
+    const server = new Service({
+      ros,
+      serviceType: 'std_srvs/Trigger',
+      name: '/test_readvertise_async'
+    });
+
+    // First advertisement
+    server.advertiseAsync(async () => {
+      return {
+        success: true,
+        message: 'first'
+      }
+    });
+
+    expect(server.isAdvertised).toBe(true);
+
+    // Re-advertise with different callback - should not throw
+    expect(() => {
+      server.advertiseAsync(async () => {
+        return {
+          success: true,
+          message: 'second'
+        }
+      });
+    }).not.toThrow();
+
+    expect(server.isAdvertised).toBe(true);
+
+    server.unadvertise();
+    expect(server.isAdvertised).toBe(false);
+  })
 })