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

Bug/tcp panic unrecovered blocking port #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hcliff
Copy link

@hcliff hcliff commented Apr 10, 2017

I kept running into an issue where running the dredd cli in rapid succession failed on account of a port being in use.

anyway, this is the resulting pr, I worked through some docs as I came to understand the project, added some logging to debug and ultimately resolved my issue.

great project, would love to see more of dredd written in go!

This shouldn't break anything, more than happy to get involved and make changes as required though

@hcliff hcliff force-pushed the bug/tcp-panic-unrecovered-blocking-port branch from 505d869 to ad9c783 Compare April 10, 2017 04:14
@IngmarStein
Copy link
Contributor

IngmarStein commented Apr 10, 2017

Hey @hcliff, did you see #23? It seems like we were running into the same issue and implemented the connection retry for the hooks server almost identically.

CC @snikch

@ddelnano
Copy link
Collaborator

sorry guys was out of town but I will look at #23 and this hopefully this week.

@hcliff
Copy link
Author

hcliff commented Apr 10, 2017

@IngmarStein yup saw it, hope you don't mind but I cribbed some code (main.go:41), while this does make it more reliable I also wanted to address the underlying cause (unclean shutdown leaving running processes)

@IngmarStein
Copy link
Contributor

Don't mind at all 👍 Did you find out why the tests are failing?

@hcliff hcliff force-pushed the bug/tcp-panic-unrecovered-blocking-port branch from ad9c783 to 29befb6 Compare April 10, 2017 14:36
@hcliff
Copy link
Author

hcliff commented Apr 10, 2017

nope, will dig in this week/end

@ddelnano
Copy link
Collaborator

@hcliff I found the issue with the tests. I pushed a fix here #25. It was related to the fact that you moved the Accept call into NewServer which blocks unless a client connects to it. There is still an issue because the refactor to cmd/goodman/main.go does not clean up after itself which is why the cucumber tests fail now. I am trying to work through that last issue.

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.

None yet

3 participants