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

[Nim] Update httpbeast to v0.3 #4554

Merged
merged 11 commits into from
Aug 18, 2021
Merged

[Nim] Update httpbeast to v0.3 #4554

merged 11 commits into from
Aug 18, 2021

Conversation

waghanza
Copy link
Collaborator

Hi @dom96,

This PR update https://github.com/dom96/httpbeast to 0.3.

On my os (osx), I could not start

Starting 4 threads
Listening on port 3000
oserr.nim(94)            raiseOSError
Error: unhandled exception: Address family not supported by protocol [OSError]

I however updated CI config so as to see any containers logs.

Hope this will be helpful


Closes #4449

@waghanza waghanza force-pushed the nim/httpbeast/v0.3 branch 2 times, most recently from e69b57f to e7f1ee4 Compare August 16, 2021 13:17
@waghanza waghanza force-pushed the nim/httpbeast/v0.3 branch from e7f1ee4 to 2ea087b Compare August 16, 2021 13:20
@dom96
Copy link
Contributor

dom96 commented Aug 16, 2021

Thanks for trying! Any chance you can try Ubuntu as the CI docker image for httpbeast?

@waghanza waghanza force-pushed the nim/httpbeast/v0.3 branch from ebc5017 to bc07407 Compare August 16, 2021 18:27
@waghanza
Copy link
Collaborator Author

@dom96
Copy link
Contributor

dom96 commented Aug 16, 2021

This is very peculiar as it works just fine in HttpBeast's CI: https://github.com/dom96/httpbeast/actions/runs/1133816001. Could it be some weird Docker-related problem? Although then it would work for you on macOS, sadly I don't have access to my MBP right now so can't test. Any chance you'd be willing to bisect httpbeast to see what introduces the bug on macOS?

@waghanza
Copy link
Collaborator Author

waghanza commented Aug 17, 2021

At first glance, the reuse port option seems troubling, but I will take a deeper look

@waghanza waghanza force-pushed the nim/httpbeast/v0.3 branch from a3cf7f0 to 25a4990 Compare August 17, 2021 07:36
@waghanza
Copy link
Collaborator Author

waghanza commented Aug 17, 2021

@dom96

Two scenarios are OK :

@dom96
Copy link
Contributor

dom96 commented Aug 17, 2021

@waghanza but did 0.3.0 stop using AF_INET6? I looked at the changes between the versions and nothing really seemed to stick out that could cause this.

@waghanza
Copy link
Collaborator Author

Indeed, ans the error stack (when disabling cli option) point to a logging cause -> to investiguate

@waghanza
Copy link
Collaborator Author

The full stack is

Starting 4 threads
Listening on port 3000
/root/.nimble/pkgs/httpbeast-0.3.0/httpbeast.nim(318) eventLoop
/nim/lib/pure/net.nim(275) newSocket
/nim/lib/pure/includes/oserr.nim(94) raiseOSError
Error: unhandled exception: Address family not supported by protocol [OSError]

@waghanza
Copy link
Collaborator Author

@dom96 I can see in dom96/httpbeast@v0.2.2...v0.3.0 that domain is set in 3.0 (Domain.AF_INET) but not in 2.0

@dom96
Copy link
Contributor

dom96 commented Aug 17, 2021

@waghanza I see, but it uses the same default as the stdlib (https://nim-lang.org/docs/net.html#newSocket%2CSocketHandle%2CDomain%2CSockType%2CProtocol). So when the value isn't specified it should still be using AF_INET.

@waghanza
Copy link
Collaborator Author

Yes @dom96.

Since, it works with AF_INET6 but not AF_INET, somehow IPv4 is disabled, but IPv6 is enabled.

However, without any custom Settings it works.

The full stack trace is

Starting 8 threads
Listening on port 3000
/Users/xxxx/.nimble/pkgs/httpbeast-0.3.0/httpbeast.nim(318) eventLoop
/usr/local/Cellar/nim/1.4.8/nim/lib/pure/net.nim(275) newSocket
/usr/local/Cellar/nim/1.4.8/nim/lib/pure/includes/oserr.nim(94) raiseOSError
Error: unhandled exception: Bad file descriptor [OSError]

@waghanza
Copy link
Collaborator Author

@dom96 The actual code works. However, I had to specify Domain.AF_INET in settings.

I do not know nim grammar, but why there is : before https://github.com/dom96/httpbeast/blob/d679e50578969f116cd1bd989e39951535a4d5bf/src/httpbeast.nim#L66
and = on this line ?

@waghanza waghanza force-pushed the nim/httpbeast/v0.3 branch from a1977be to 48e6916 Compare August 17, 2021 19:53
@dom96
Copy link
Contributor

dom96 commented Aug 17, 2021

I do not know nim grammar, but why there is : before https://github.com/dom96/httpbeast/blob/d679e50578969f116cd1bd989e39951535a4d5bf/src/httpbeast.nim#L66
and = on this line ?

It's the return type of the procedure. In most cases : is followed by a type in Nim.

@dom96
Copy link
Contributor

dom96 commented Aug 17, 2021

I still am quite confused by this problem. The default should be Domain.AF_INET so why does specifying it in settings explicitly work?

@waghanza
Copy link
Collaborator Author

I still am quite confused by this problem.

Me too

@waghanza
Copy link
Collaborator Author

@dom96 without specifying domain, the default socket type is AF_UNSPEC, and without specifying settings the domain is AF_INET

@waghanza waghanza enabled auto-merge (squash) August 18, 2021 09:19
@@ -11,4 +11,4 @@ proc onRequest(req: Request): Future[void] =
if req.path.get() == "/user":
req.send(Http200, "")

run(onRequest, Settings(port: Port(3000)))
run(onRequest, Settings(port: Port(3000), domain: Domain.AF_INET))
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see. Well here is your bug, please use initSettings instead which will set the defaults properly.

Copy link
Collaborator Author

@waghanza waghanza Aug 18, 2021

Choose a reason for hiding this comment

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

oh I see ... https://github.com/dom96/httpbeast/blob/b33dba93a134faa2adc3b991cb69890375987076/src/httpbeast.nim#L464 should not be used directly unless using initSettings

however, should not httpbeast use AF_UNSPEC so as either IPv6 or IPv4 is determined on nim level (depending on system ability) ?

Copy link
Collaborator Author

@waghanza waghanza Aug 18, 2021

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

however, should not httpbeast use AF_UNSPEC so as either IPv6 or IPv4 is determined on nim level (depending on system ability) ?

not sure what you mean. You mean it should automatically choose IPv6 or IPv4 if AF_UNSPEC is used? That's not something Nim supports

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exactly @dom96. I think to follow standards httpbeast should use AF_UNSPEC as long as nim should choose IPv4 or IPv6 depending on system abilities https://man7.org/linux/man-pages/man3/getaddrinfo.3.html

Copy link
Contributor

Choose a reason for hiding this comment

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

@waghanza but that's exactly what doesn't work, right? Maybe it's a problem on the Nim side, is that what you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. The network / transport layer is, in my opinion, a language concern.

@dom96
Copy link
Contributor

dom96 commented Aug 18, 2021

Thanks for looking into it!

@waghanza waghanza disabled auto-merge August 18, 2021 13:58
@waghanza waghanza merged commit ab7a7f6 into master Aug 18, 2021
@waghanza waghanza deleted the nim/httpbeast/v0.3 branch August 18, 2021 13:58
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.

[Nim] Update httpbeast to 0.3
2 participants