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

Fail on schema response HTTP error #313

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

mrstif
Copy link
Contributor

@mrstif mrstif commented Apr 3, 2018

When running neo4j:migrate and the call returns an unexpected error, e.g. (502 - Bad Gateway), the following error is returned:

neo4j-core-8.1.2/lib/neo4j/core/cypher_session/adaptors/http.rb:69 constraints
NoMethodError: undefined method `map' for #String:0x000000000917c6a0

This is due to the fact that only structured errors are being handled.
In order to facilitate debugging, this PR fails on any HTTP error that might occur during that process.

I'm not sure if the :errors key can be present even if the result is a success, so i left the logic separated.

Pings:
@cheerfulstoic
@subvertallchris

@mrstif mrstif force-pushed the fail_on_schema_http_error branch from 60be9ed to 9e1e95c Compare April 3, 2018 13:42
@coveralls
Copy link

coveralls commented Apr 3, 2018

Coverage Status

Coverage decreased (-0.01%) to 74.533% when pulling 9e1e95c on mrstif:fail_on_schema_http_error into 431d609 on neo4jrb:master.

@cheerfulstoic
Copy link
Contributor

Seems reasonable, thanks for this!

@cheerfulstoic cheerfulstoic merged commit 0ae26e8 into neo4jrb:master Apr 4, 2018
cheerfulstoic pushed a commit that referenced this pull request Apr 4, 2018
…or to avoid obscure errors afterwards (#313)

(cherry picked from commit 0ae26e8)
cheerfulstoic added a commit that referenced this pull request Apr 4, 2018
@cheerfulstoic
Copy link
Contributor

Released as 8.1.3

@mrstif mrstif deleted the fail_on_schema_http_error branch April 4, 2018 12:10
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.

3 participants