Rollback calls failing silently after invalid pack error

[kpdecker]

When rolling back after this root error:

Cannot pack this value: undefined

the promise fails to reject or resolve:

      return tx.rollback()
        .then(() => {
          console.log('resolve!');
          session.close();
          operations = tx = session = undefined;
        },
        (err) => {
          console.log('reject!', err);
          session.close();
          operations = tx = session = undefined;

          log(['error', 'neo', 'transaction', 'rollback'], {sourceId, transactionId, error: err.stack});
          return Promise.reject(err);
        });

Trying to log through this a bit, I found the following:

packStruct 10
Error
    at Packer.packStruct (~/node_modules/neo4j-driver/lib/v1/internal/packstream.js:160:21)
    at Connection.run (~/node_modules/neo4j-driver/lib/v1/internal/connector.js:384:20)
    at _runDiscardAll (~/node_modules/neo4j-driver/lib/v1/transaction.js:236:8)
    at Object.rollback (~/node_modules/neo4j-driver/lib/v1/transaction.js:168:24)
    at Transaction.rollback (~/node_modules/neo4j-driver/lib/v1/transaction.js:115:35)
    at Object.rollback (~/src/neo4j.js:144:25)
    at ~/src/neo4j.js:103:31
    at lib$es6$promise$$internal$$tryCatch (~/node_modules/neo4j-driver/lib/external/es6-promise.js:338:14)
    at lib$es6$promise$$internal$$invokeCallback (~/node_modules/neo4j-driver/lib/external/es6-promise.js:353:15)
    at lib$es6$promise$$internal$$publish (~/node_modules/neo4j-driver/lib/external/es6-promise.js:321:9)
    at lib$es6$promise$$internal$$publishRejection (~/node_modules/neo4j-driver/lib/external/es6-promise.js:263:5)
    at lib$es6$promise$asap$$flush (~/node_modules/neo4j-driver/lib/external/es6-promise.js:120:7)
    at doNTCallback0 (node.js:430:9)
    at process._tickDomainCallback (node.js:400:13)
pack ROLLBACK string
pack {} object
packStruct 2f

To which the server responded:

message! Structure { signature: 112, fields: [ { fields: [] } ] }
message! Structure { signature: 112, fields: [ {} ] }
message! Structure {
  signature: 127,
  fields:
   [ { code: 'Neo.ClientError.Request.InvalidFormat',
       message: 'Unknown struct type: 10' } ] }

Client Instance:

$ node --version
v5.1.1
$ npm ls | grep neo
├── [email protected]

Database Instance:

neo4j:
  container_name: neo4j
  image: neo4j:3.0.2-enterprise
  ports:
    - "7474:7474"
    - "7687:7687"

So after all of that, my concerns are:

1. It seems that the pack algorithm blows up when undefined is passed in a parameters object. It seems like this should silently ignore this case.
2. I fear that there are other possible cases that error handling can fail when using the promises form of requests.

[pontusmelke]

Thanks for reporting, will investigate this.

Regards
Pontus

[pontusmelke]

I have merged your PR now, are you fine with closing this issue now @kpdecker ?

The issue of gracefully failing when the packer sees a type it doesn't know about is a bigger issue since it may already have written to the channel so it could be in a weird state. I'd like to think about that a little before we do something about it.

[kpdecker]

Ideally there would be some sanity checks for corruption of the binary protocol, but I'm not sure of an easy way to achieve this. The initial issue is fixed though, so your call on closing.

[pontusmelke]

@kpdecker Yeah I will try to fix this properly, soonish so I'll leave it open