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 domains #3573

Closed
hueniverse opened this issue Sep 15, 2017 · 10 comments
Closed

Remove domains #3573

hueniverse opened this issue Sep 15, 2017 · 10 comments
Assignees
Labels
breaking changes Change that can breaking existing code
Milestone

Comments

@hueniverse
Copy link
Contributor

No description provided.

@hueniverse hueniverse added the breaking changes Change that can breaking existing code label Sep 15, 2017
@hueniverse hueniverse added this to the 17.0.0 milestone Sep 15, 2017
@hueniverse hueniverse self-assigned this Sep 15, 2017
@dwelle
Copy link
Contributor

dwelle commented Sep 19, 2017

Means we'll have to catch exceptions in prereqs, plugins, route handlers ourselves?

@devinivy
Copy link
Member

devinivy commented Sep 19, 2017

Not exactly– the idea is to use async/await as the foundation for error handling rather than domains.

@kanongil
Copy link
Contributor

Domains is not intended for normal error handling. It is mainly there to catch unexpected issues that can appear in production, giving you a chance to handle it gracefully.

@dwelle
Copy link
Contributor

dwelle commented Sep 19, 2017

@kanongil yes, that's what I meant... whether I should start worrying that unhandled exceptions will start killing our servers.

But I guess I was wrong as per @devinivy

@devinivy
Copy link
Member

The approach has limitations– there is an ongoing conversation about it if you have any thoughts :)

@mtharrison
Copy link
Contributor

@devinivy where is the conversation? Was there any reasonable alternative proposed for how to deal with uncaught exceptions bringing your app down?

@devinivy
Copy link
Member

At the time I wrote that there was chatter in here and on the hapi hour slack. I was anticipating more conversation on this issue, though. Now that hapi is based on async/await in v17, the majority of runtime exceptions (including those that would formerly be uncaught) should turn into rejections that the framework will handle for you. As for the exceptions that cannot be handled this way, there is no proposed solution that I know of. It was noted in hapi hour that when node's async-hooks API becomes stable, that might be a viable option.

@hueniverse
Copy link
Contributor Author

@mtharrison there are no good solutions for the edge cases where a handler throws inside another tick (e.g. you throw inside a timer). Even domains didn't really solve the problem of throwing in every resource pool and some people had to manually bind the request.domain to their handlers. It was a mess to support with monthly new issues about domains not playing well with something.

I've decided the best path forward is to go all in on async/await and let people decide what they want to do about calling code in their handlers that doesn't follow the promises contract. I looked into bringing domains back into v17 but I just don't want to add that for the few edge cases left.

@mtharrison
Copy link
Contributor

@hueniverse that makes sense. It was an imperfect solution to the problem anyway but caused lots of trouble. Thanks for the clarification.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking changes Change that can breaking existing code
Projects
None yet
Development

No branches or pull requests

5 participants