v2 / Errors: improve impl. and tests.

API_v2.md

@@ -305,11 +305,11 @@ using `Error` subclasses. Importantly:
 
 - Special care is taken to provide `message` and `stack` properties rich in
  info, so that no special serialization is needed to debug production errors.
+ That is, simply logging the `stack` (as most error handlers tend to do)
+ should get you meaningful and useful data.
 
 - Structured info returned by Neo4j is also available on the `Error` instances
  under a `neo4j` property, for deeper introspection and analysis if desired.
- In addition, if this error is associated with a full HTTP response, the HTTP
- `statusCode`, `headers`, and `body` are available via an `http` property.
 
 ```coffee
 class Error {name, message, stack, http, neo4j}

lib-new/GraphDatabase.coffee

@@ -1,5 +1,5 @@
 $ = require 'underscore'
-errors = require './errors'
+{Error} = require './errors'
 lib = require '../package.json'
 Node = require './Node'
 Relationship = require './Relationship'
@@ -55,20 +55,10 @@ module.exports = class GraphDatabase
 
  {body, headers, statusCode} = resp
 
- if statusCode >= 500
- # TODO: Parse errors, and differentiate w/ TransientErrors.
- err = new errors.DatabaseError 'TODO',
- http: {body, headers, statusCode}
+ if err = Error._fromResponse resp
  return cb err
 
- if statusCode >= 400
- # TODO: Parse errors.
- err = new errors.ClientError 'TODO',
- http: {body, headers, statusCode}
- return cb err
-
- # Parse nodes and relationships in the body, and return:
- return cb null, _transform body
+ cb null, _transform body
 
 
 ## HELPERS

lib-new/errors.coffee

@@ -1,13 +1,41 @@
 $ = require 'underscore'
+http = require 'http'
 
 class @Error extends Error
 
- Object.defineProperties @::,
- name: {get: -> 'neo4j.' + @constructor.name}
-
- constructor: (@message='Unknown error', {@http, @neo4j}={}) ->
+ constructor: (@message='Unknown error', @neo4j={}) ->
+ @name = 'neo4j.' + @constructor.name
  Error.captureStackTrace @, @constructor
 
+ #
+ # Accepts the given HTTP client response, and if it represents an error,
+ # creates and returns the appropriate Error instance from it.
+ # If the response doesn't represent an error, returns null.
+ #
+ @_fromResponse: (resp) ->
+ {body, headers, statusCode} = resp
+
+ return null if statusCode < 400
+
+ ErrorType = if statusCode >= 500 then 'Database' else 'Client'
+ ErrorClass = exports["#{ErrorType}Error"]
+
+ message = "#{statusCode} "
+ logBody = statusCode >= 500 # TODO: Config to always log body?
+
+ if body?.exception
+ message += "#{body.exception}: #{body.message or '(no message)'}"
+ else
+ statusText = http.STATUS_CODES[statusCode] # E.g. "Not Found"
+ reqText = "#{resp.request.method} #{resp.request.path}"
+ message += "#{statusText} response for #{reqText}"
+ logBody = true # always log body if non-error returned
+
+ if logBody and body?
+ message += ": #{JSON.stringify body, null, 4}"
+
+ new ErrorClass message, body
+
  # TODO: Helper to rethrow native/inner errors? Not sure if we need one.
 
 class @ClientError extends @Error

test-new/http._coffee

@@ -45,6 +45,19 @@ expectNeo4jRoot = (body) ->
  expect(body).to.be.an 'object'
  expect(body).to.have.keys 'data', 'management'
 
+#
+# Asserts that the given object is a proper instance of the given Neo4j Error
+# subclass, including with the given message.
+# Additional checks, e.g. of the `neo4j` property's contents, are up to you.
+#
+expectError = (err, ErrorClass, message) ->
+ expect(err).to.be.an.instanceOf ErrorClass
+ expect(err.name).to.equal "neo4j.#{ErrorClass.name}"
+ expect(err.neo4j).to.be.an 'object'
+ expect(err.message).to.equal message
+ expect(err.stack).to.contain '\n'
+ expect(err.stack.split('\n')[0]).to.equal "#{err.name}: #{err.message}"
+
 
 ## TESTS
 
@@ -64,21 +77,19 @@ describe 'GraphDatabase::http', ->
 
  expectNeo4jRoot body
 
- it 'should throw errors for 4xx responses by default', (_) ->
- try
- thrown = false
- DB.http
- method: 'POST'
- path: '/'
- , _
- catch err
- thrown = true
- expect(err).to.be.an.instanceOf neo4j.ClientError
- expect(err.name).to.equal 'neo4j.ClientError'
- expect(err.http).to.be.an 'object'
- expect(err.http.statusCode).to.equal 405
-
- expect(thrown).to.be.true()
+ it 'should throw errors for 4xx responses by default', (done) ->
+ DB.http
+ method: 'POST'
+ path: '/'
+ , (err, body) ->
+ expect(err).to.exist()
+ expect(body).to.not.exist()
+
+ expectError err, neo4j.ClientError,
+ '405 Method Not Allowed response for POST /'
+ expect(err.neo4j).to.be.empty()
+
+ done()
 
  it 'should support returning raw responses', (_) ->
  resp = DB.http
@@ -100,23 +111,27 @@ describe 'GraphDatabase::http', ->
 
  expectResponse resp, 405 # Method Not Allowed
 
- it 'should throw native errors always', (_) ->
+ it 'should throw native errors always', (done) ->
  db = new neo4j.GraphDatabase 'http://idontexist.foobarbaz.nodeneo4j'
+ db.http
+ path: '/'
+ raw: true
+ , (err, resp) ->
+ expect(err).to.exist()
+ expect(resp).to.not.exist()
 
- try
- thrown = false
- db.http
- path: '/'
- raw: true
- , _
- catch err
- thrown = true
+ # NOTE: *Not* using `expectError` here, because we explicitly don't
+ # wrap native (non-Neo4j) errors.
  expect(err).to.be.an.instanceOf Error
  expect(err.name).to.equal 'Error'
  expect(err.code).to.equal 'ENOTFOUND'
  expect(err.syscall).to.equal 'getaddrinfo'
+ expect(err.message).to.contain "#{err.syscall} #{err.code}"
+ # NOTE: Node 0.11 adds the hostname to the message.
+ expect(err.stack).to.contain '\n'
+ expect(err.stack.split('\n')[0]).to.equal "#{err.name}: #{err.message}"
 
- expect(thrown).to.be.true()
+  done()
 
  it 'should support streaming'
  # Test that it immediately returns a duplex HTTP stream.