Skip to content

Conversation

noah-wardlow
Copy link

  • Allow re-advertisement: advertise() and
    advertiseAsync() now automatically unadvertise before
    re-advertising instead of throwing errors
  • Make unadvertise idempotent: unadvertise() now
    safely returns instead of throwing when called on
    non-advertised services
  • Fix cleanup order: Callbacks are removed before
    marking as unadvertised to prevent processing
    requests during cleanup

Copy link
Member

@EzraBrooks EzraBrooks left a comment

Choose a reason for hiding this comment

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

Firstly I think we should make this PR upstream

But secondly I think this might be too fragile. I added those throws because before they were there this failed silently due to race conditions, so I made it fail hard instead.

This whole library is built around eventemitters - is there some way we can debounce, queue, throttle, or otherwise lock the advertisement to ensure no race conditions can occur here?

@noah-wardlow
Copy link
Author

RobotWebTools#919

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants