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

Attempt to fix #3399 where it crashes on route prerequisites when no domain is present #3401

Merged
merged 1 commit into from Dec 1, 2016

Conversation

@sirgallifrey
Copy link
Contributor

@sirgallifrey sirgallifrey commented Dec 1, 2016

I wrote a failing test witch match the description on #3399 and added a check to see if domain exists before binding. I also changed domain = process.domain to domain = request.domain to prevent the test domain from being used.

I'm sorry in advance if this fix is no good, I just recently started to studying the hapi internals

@sirgallifrey sirgallifrey changed the title Attempt to fix #3399 here it crashes on route prerequisites when no domain is present Attempt to fix #3399 where it crashes on route prerequisites when no domain is present Dec 1, 2016
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Dec 1, 2016

Don't apologize in advance (or after the fact for that matter) for making a mistake. I don't know if you have, I'm just saying in general. I appreciate the effort. I'll review this week.

@simlevesque
Copy link
Contributor

@simlevesque simlevesque commented Dec 1, 2016

I'll be able to test this PR on my machine environment later today.

@AdriVanHoudt
Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Dec 1, 2016

@sirgallifrey yeah no need to say sorry, we appreciate every contribution very much!

@simlevesque
Copy link
Contributor

@simlevesque simlevesque commented Dec 1, 2016

This PR solves the bug ! Not much else to say, it used not to work, now it does.

Thank you very much everyone.

@hueniverse hueniverse added the bug label Dec 1, 2016
@hueniverse hueniverse added this to the 16.0.1 milestone Dec 1, 2016
@hueniverse hueniverse self-assigned this Dec 1, 2016
@hueniverse hueniverse merged commit 47a9566 into hapijs:master Dec 1, 2016
2 checks passed
@lock
Copy link

@lock 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants