Latest versions won't accept custom event hubs errors

[pierreca]

Some custom event hubs AMQP errors (such as com.microsoft:argument-out-of-range when trying to connect to a non-existing partition) get swallowed. This is a breaking change, and as far as I can tell there's no way for the client to know what happened, the connection is just killed with an EncodingError that masks the real cause.
This is due to the changes in connection.js in this commit: 5455179

Also, it seems there's an issue with the way error.js is required in types/amqp_error.js, which leads to the throw on line 8 to throw a TypeError: errors.EncodingError is not a constructor instead of an actual EncodingError (at least, this happens with node 6.
Easiest fix for this is to use module.exports. wherever errors. is used in errors.js and get rid of the local errors variable alltogether.. not elegant but it works...

Unfortunately this is blocking the Event Hubs SDK from upgrading to the latest version of the library :-/

[mbroadst]

@pierreca can you explain why you think the error was introduced there? To me it seems far more likely that an error frame is being received, however it has an unknown ErrorCondition and therefore we're not wrapping it properly. It would be incredibly helpful to have e.g. the hex for the data representing that error frame so we could create a unit test for it. The EncodingError makes me feel like something more nefarious is going on, but it might be data accidentally left in the buffer as well. Do you have a way to reproduce this in a unit test?

[mbroadst]

@pierreca ah sorry I found the culprit, you're talking about this: https://github.com/noodlefrenzy/node-amqp10/blob/master/lib/types/amqp_error.js#L7. Yeah that totally looks like a bug if MS is using custom errors, I had read the spec to indicate that they only wanted to support the listed error conditions. Again it would be very helpful if you could provide a sample custom error, we can get this pushed in today

[mbroadst]

@pierreca for your local testing, it might be enough to replace the throw new errors.EncodingError... there with return value;

[pierreca]

Here's what I believe is the sample error trace for the unit test:

0000013b02000000005316d00000012b00000003434100531dd00000011d00000003a323636f6d2e6d6963726f736f66743a617267756d656e742d6f75742d6f662d72616e6765a1f15468652073706563696669656420706172746974696f6e20697320696e76616c696420666f7220616e204576656e7448756220706172746974696f6e2073656e646572206f722072656365697665722e2049742073686f756c64206265206265747765656e203020616e6420372e0d0a506172616d65746572206e616d653a20506172746974696f6e496420547261636b696e6749643a66393365393966643264666234366461383366353662633133633933636162635f47332c2053797374656d547261636b65723a67617465776179362c2054696d657374616d703a322f372f3230313720373a31383a303120504d40

And a full trace:

amqp10:link undefined:0: Transitioning from DETACHED to ATTACHING due to sendAttach +10ms
  amqp10:link attach CH=1, Handle=0 +14ms
  amqp10:framing sending frame:  @attach(18) [name='/ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0' handle=0 role=true sndSettleMode=2 rcvSettleMode=0 source=@source(40) [address='/ingress/ConsumerGroups/$Default/Partitions/bad' durable=0 expiryPolicy='session-end' timeout=0 dynamic=false dynamicNodeProperties={} distributionMode=null filter={} defaultOutcome=null outcomes=null capabilities=null] target=@target(41) [address='localhost' durable=0 expiryPolicy='session-end' timeout=0 dynamic=false dynamicNodeProperties={} capabilities=null] unsettled={} incompleteUnsettled=false initialDeliveryCount=1 maxMessageSize=0 offeredCapabilities=null desiredCapabilities=null properties={}] +6ms
  amqp10:trace raw: [000000ec02000001005312c0df0ea1542f696e67726573732f436f6e73756d657247726f7570732f2444656661756c742f506172746974696f6e732f6261645f35636663363334642d336437392d343938662d613966352d333065623134653530356530434150025000005328c04c0ba12f2f696e67726573732f436f6e73756d657247726f7570732f2444656661756c742f506172746974696f6e732f62616443a30b73657373696f6e2d656e644342c1010040c10100404040005329c02007a1096c6f63616c686f737443a30b73657373696f6e2d656e644342c1010040c10100425201444040c10100] +14ms
  amqp10:connection Rx: 0000007d02000000005312c0700ea1542f696e67726573732f436f6e73756d657247726f7570732f2444656661756c742f506172746974696f6e732f6261645f35636663363334642d336437392d343938662d613966352d33306562313465353035653043425002500040404040438000000000000410004040c10100 +322ms
  amqp10:framing received frame:  @attach(18) [name='/ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0' handle=0 role=false sndSettleMode=2 rcvSettleMode=0 source=null target=null unsettled={} incompleteUnsettled=false initialDeliveryCount=0 maxMessageSize=266240 offeredCapabilities=null desiredCapabilities=null properties={}] +2ms
  amqp10:trace raw: [005312c0700ea1542f696e67726573732f436f6e73756d657247726f7570732f2444656661756c742f506172746974696f6e732f6261645f35636663363334642d336437392d343938662d613966352d33306562313465353035653043425002500040404040438000000000000410004040c10100] +2ms
  amqp10:link /ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0:0: Transitioning from ATTACHING to ATTACHED due to attachReceived +4ms
  amqp10:link /ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0: attached CH=[1=>0], Handle=[0=>0] +1ms
  amqp10:policy:utils Refreshing link /ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0 credit by 100: 0 remaining. +22ms
  amqp10:link:receiver addCredits (/ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0): New values: link(100), session(200) +1ms
  amqp10:framing sending frame:  @flow(19) [nextIncomingId=1 incomingWindow=200 nextOutgoingId=1 outgoingWindow=100 handle=0 deliveryCount=0 linkCredit=100 available=0 drain=false echo=false properties={}] +16ms
  amqp10:trace raw: [0000002002000001005313c0130b520152c85201526443435264434242c10100] +5ms
  amqp10:connection Rx: 0000013c02000000005316d00000012c00000003434100531dd00000011e00000003a323636f6d2e6d6963726f736f66743a617267756d656e742d6f75742d6f662d72616e6765a1f25468652073706563696669656420706172746974696f6e20697320696e76616c696420666f7220616e204576656e7448756220706172746974696f6e2073656e646572206f722072656365697665722e2049742073686f756c64206265206265747765656e203020616e6420372e0d0a506172616d65746572206e616d653a20506172746974696f6e496420547261636b696e6749643a63323138653336316135353234633966616466343633316132646165323765635f4734372c2053797374656d547261636b65723a67617465776179362c2054696d657374616d703a322f372f3230313720373a32373a323020504d40 +81ms
  amqp10:client connection error:  TypeError: errors.EncodingError is not a constructor
    at Object.errorCondition [as requires] (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\types\amqp_error.js:8:11)
    at wrapField (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\types\composite_type.js:32:47)
    at Object.set [as condition] (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\types\composite_type.js:66:48)
    at Object.Composite (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\types\composite_type.js:86:26)
    at convertKnownType (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\types\composite_type.js:14:12)
    at Array.map (native)
    at Composite (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\types\composite_type.js:83:48)
    at Object.frames.readFrame (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\frames.js:117:15)
    at Connection._receiveAny (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\connection.js:409:22)
    at Connection._receiveData (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\connection.js:357:8)
    at .<anonymous> (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\connection.js:516:38)
    at emitOne (events.js:96:13)
    at emit (events.js:188:7)
    at TLSSocket.<anonymous> (C:\github\azure-event-hubs-node\send_receive\node_modules\amqp10\lib\transport\tls_transport.js:29:49)
    at emitOne (events.js:96:13)
    at TLSSocket.emit (events.js:188:7)
    at readableAddChunk (_stream_readable.js:172:18)
    at TLSSocket.Readable.push (_stream_readable.js:130:10)
    at TLSWrap.onread (net.js:542:20) +4ms
  amqp10:framing sending frame:  @close(24) [error={ condition: 'connection:framing-error',
  description: 'errors.EncodingError is not a constructor' }] +3ms
  amqp10:trace raw: [0000006102000000005318c0540100531dc04e03a31d616d71703a636f6e6e656374696f6e3a6672616d696e672d6572726f72a1296572726f72732e456e636f64696e674572726f72206973206e6f74206120636f6e7374727563746f72c10100] +21ms
  amqp10:client disconnected +13ms
  amqp10:link force detach for /ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0. current state: attached +8ms
  amqp10:link /ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0:0: Transitioning from ATTACHED to DETACHED due to forceDetach +5ms
  amqp10:session detached(/ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0): AmqpProtocolError: amqp:link:detach-forced:detach-forced +18ms
  amqp10:session removing receiver link /ingress/ConsumerGroups/$Default/Partitions/bad_5cfc634d-3d79-498f-a9f5-30eb14e505e0 +2ms