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

remove use of [core] domain module - due to it being deprecated #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iamjochem
Copy link

the domain module is marked as deprecated (see here), I think it is wise to remove it.

Removing it's usage does not have any effect on normal shutdown operation.

Given that uncaught exceptions should be treated as a stop-the-world situation and we are already shutting down in this context I feel that no love is lost.

What do you think?

@suprememoocow
Copy link
Member

Thanks for the PR, but one of the tests is failing:

  1) shutdown-coordinator should deal with stages that error:
     Uncaught
  Error
      at Timeout._onTimeout (tests/shutdown-coordinator-test.js:60:15)
      at ontimeout (timers.js:365:14)
      at tryOnTimeout (timers.js:237:5)
      at Timer.listOnTimeout (timers.js:20

This test specific tests the case where a shutdown handler throws an unhandled/uncaught exception. Because we're running this code from inside a domain, we're able to recover from it. Offhand, I'm not sure what the best non-domain alternative for this situation is.

@suprememoocow suprememoocow force-pushed the feature/remove-domain-module branch from e4c29fb to b1b1c5d Compare April 27, 2017 11:29
@iamjochem iamjochem force-pushed the feature/remove-domain-module branch from b1b1c5d to ba4906d Compare August 22, 2017 13:40
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