Skip to content

Failed to read from defunct connection Address(host='127.0.0.1', port=7687) #260

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

Closed
wbleker opened this issue Sep 27, 2018 · 4 comments
Closed

Comments

@wbleker
Copy link

wbleker commented Sep 27, 2018

We are having issues with the python bolt driver and how it handles working with key value pairs, in our example the key is an int, in 1.5.3 it works, but in 1.6.x and 1.7.x it fails.

In the repo below is an example script that can be used with different driver versions to reproduce the error.

https://github.com/wbleker/neo4j_bolt_test.git

@technige
Copy link
Contributor

technige commented Oct 4, 2018

Numeric keys are not supported in Cypher maps; keys must always be strings. That it previously worked would have likely been an accident of implementation.

@technige technige closed this as completed Oct 4, 2018
@sproberts92
Copy link

Hi, colleague of @wbleker here.

I don't think this can be closed yet. I think the original issue can be decomposed into two separate issues, the second of which warrants further attention:

  1. The integer key in the map is not accepted - fair enough to make that choice (in line with, for example, the json standard) but it seems that in 1.5.3, this wasn't completely accidental. Commit dc1762f changes from ustr(key) to just key in the dehydration function without explanation (first introduced in Parameter type checking #200). Perhaps you can share the motivation in that change? Although integer keys are valid in a Python dict, and there is merit in the driver handling the coercion here, I understand that problems can arise trying to use both 1 and '1' as keys. If that's the reason then, alright. Perhaps some comment should be added to the documentation or release notes? This 'bug fix' is still a breaking change moving above 1.5.3.

  2. The error message produced is not meaningful. This seems to be unrelated to the specifics of the query. In versions <=1.5.3 we get back a meaningful error. For example if I send the query abc, I get

Invalid input 'a': expected <init> (line 1, column 1 (offset: 0))
"abc"
 ^

but post 1.5.3 I only get the error in mentioned in the original issue

Failed to read from defunct connection Address(host='::1', port=7687, flow_info=0, scope_id=0)

so something has changed in the error handling after 1.5.3.

I think (1) can be argued as being correct behaviour but (2) seems to need further attention.

@technige
Copy link
Contributor

technige commented Oct 4, 2018

Hi @sproberts92

The change from ustr(key) to key that you note was to prevent implicit coercion of key values to strings. As you suggest, a dictionary such as {1: 'foo', '1': 'bar'} would collapse in that case. The public surface has been written with the expectation that applications adhere to the Cypher type system as set out in the manual; in this case specifically, string keys are mentioned here: https://neo4j.com/docs/developer-manual/preview/cypher/syntax/values/#composite-types.

That said, I don't think we have highlighted the characteristics of this data type strongly enough. To that end, I have just spoken with our Cypher team, who are going to find a way to make this stand out in the manual more clearly. In addition, I've added an extra card to our Drivers wall to raise a TypeError if illegal key values are provided as, you are right, a defunct connection is incredibly unhelpful.

Thanks for raising this.

Nigel

@technige technige reopened this Oct 4, 2018
@technige
Copy link
Contributor

technige commented Nov 1, 2018

Fixed by #270

@technige technige closed this as completed Nov 1, 2018
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

No branches or pull requests

3 participants