Skip to content

Connection Hint: supporting connection.recv_timeout_seconds #761

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

bigmontz
Copy link
Contributor

This value defines the time the client should wait for a response on a connection before it times out.

Co-authored-by: Rouven Bauer rouven.bauer@neotechnology.com

Comment on lines 209 to 210
const receiveTimeoutInSeconds = toNumber(receiveTimeoutRaw)
if (receiveTimeoutInSeconds > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if even feasible in JS's limited number type, but we also don't expect floats from the server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will create tests with float number to limit this option. One question, should 3.0 be considered an invalid value?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know how the JS drivers handles numbers internally. But the server is expected to sent an Integer (strongly typed through packstream). It might well be, however, that at this point in the code, the type information is already lost. In that case, I'd say 3.0 is also ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At that point, I already lost this information. Since 3 is the same as 3.0 for JS. So, I'm verifying if it's a Integer using the JS Number definition of integer.

@bigmontz bigmontz force-pushed the 4.3-connection-hint-receive_timeout_seconds branch from 4ded350 to da448b8 Compare July 23, 2021 15:20
@bigmontz bigmontz force-pushed the 4.3-connection-hint-receive_timeout_seconds branch from da448b8 to ad7ec10 Compare July 26, 2021 08:57
@bigmontz bigmontz marked this pull request as ready for review July 27, 2021 12:08
@robsdedude robsdedude self-requested a review July 27, 2021 12:36
Copy link
Member

@robsdedude robsdedude left a comment

Choose a reason for hiding this comment

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

💻

@bigmontz bigmontz merged commit 34a16ca into neo4j:4.3 Jul 27, 2021
@bigmontz bigmontz deleted the 4.3-connection-hint-receive_timeout_seconds branch July 27, 2021 13:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants