search

TritonDataCenter / node-manta

Node.js SDK for Manta
75 stars 54 forks source link

"AssertionError: undefined (object) is required" after "socket hang up" #261

Closed trentm closed 8 years ago

trentm commented 8 years ago

I've seen this a few times on mrm -r of a dir with many entries. E.g.:

$ mrm -rv /trent.mick/jobs/2c799de7-bdea-e8c7-cc16-8b65850081e2
...
{"name":"mrm","hostname":"danger0.local","pid":645,"component":"MantaClient","level":10,"err":{"message":"socket hang up","name":"Error","stack":"Error: socket hang up\n    at SecurePair.error (tls.js:1011:23)\n    at EncryptedStream.CryptoStream._done (tls.js:703:22)\n    at CleartextStream.read [as _read] (tls.js:499:24)\n    at CleartextStream.Readable.read (_stream_readable.js:341:10)\n    at EncryptedStream.onCryptoStreamFinish (tls.js:304:47)\n    at EncryptedStream.g (events.js:180:16)\n    at EncryptedStream.emit (events.js:117:20)\n    at finishMaybe (_stream_writable.js:360:12)\n    at endWritable (_stream_writable.js:367:3)\n    at EncryptedStream.Writable.end (_stream_writable.js:345:5)","code":"ECONNRESET"},"msg":"Request failed","time":"2016-06-29T16:44:27.176Z","src":{"file":"/Users/trentm/joy/node-manta/node_modules/restify-clients/lib/HttpClient.js","line":230,"func":"onError"},"v":0}
mrm: AssertionError: undefined (object) is required

where that Bunyan log line is:

[2016-06-29T16:44:27.176Z] TRACE: mrm/MantaClient/645 on danger0.local (/Users/trentm/joy/node-manta/node_modules/restify-clients/lib/HttpClient.js:230 in onError): Request failed (err.code=ECONNRESET)
    Error: socket hang up
        at SecurePair.error (tls.js:1011:23)
        at EncryptedStream.CryptoStream._done (tls.js:703:22)
        at CleartextStream.read [as _read] (tls.js:499:24)
        at CleartextStream.Readable.read (_stream_readable.js:341:10)
        at EncryptedStream.onCryptoStreamFinish (tls.js:304:47)
        at EncryptedStream.g (events.js:180:16)
        at EncryptedStream.emit (events.js:117:20)
        at finishMaybe (_stream_writable.js:360:12)
        at endWritable (_stream_writable.js:367:3)
        at EncryptedStream.Writable.end (_stream_writable.js:345:5)

The code path hitting that assertion should be corrected. I'm not sure if that is getting in the way of retries.

davepacheco commented 8 years ago

We observed this internally under OPS-2074, where this resulted in a lackey crash like this:

Uncaught AssertionError: undefined (object) is required

FROM
_toss (/opt/marlin/node_modules/manta/node_modules/assert-plus/assert.js:22:11)
Function.out.(anonymous function) [as object] (/opt/marlin/node_modules/manta/node_modules/assert-plus/assert.js:122:17)
readError (/opt/marlin/node_modules/manta/lib/client.js:387:12)
ClientRequest.onResult (/opt/marlin/node_modules/manta/lib/client.js:329:13)
ClientRequest.g (events.js:180:16)
ClientRequest.emit (events.js:98:17)
_emitResult (/opt/marlin/node_modules/manta/node_modules/restify-clients/lib/HttpClient.js:187:14)
f (/opt/marlin/node_modules/manta/node_modules/once/once.js:17:25)
/opt/marlin/node_modules/manta/node_modules/restify-clients/lib/HttpClient.js:238:17
process._tickDomainCallback (node.js:463:13)
[ Sep 12 09:16:20 Stopping because process dumped core. ]

From the core file, I found this JavaScript stack frame where we blew the assertion:

js:     readError
          file: /opt/marlin/node_modules/manta/lib/client.js
          posn: line 385 
          this: a0434701 (<unknown>)
          arg1: 948a5a65 (JSObject: Error)
          arg2: a0408081 (Oddball: "null")
          arg3: 948a6481 (JSFunction)

The error object is this one:

> 948a5a65::jsprint    
{
    "arguments": undefined,
    "type": undefined,
    "message": "socket hang up",
    "code": "ECONNRESET",
}

So there's our "socket hang up". The readError() function that blew the assertion looks like this:

> 8fcd9379::jssource -n0
file: /opt/marlin/node_modules/manta/lib/client.js

  385 function readError(err, res, cb) {
  386     assert.object(err);
  387     assert.object(res);
  388     assert.func(cb);
  389 
  390     if (res === null)
  391         return (cb(null, err, res));
...

We blew the assertion at line 387, but the code obviously handles the case that res is null. Indeed, from the core file we can see that res was null.

What happened here is that before node-manta@3.0.0 (and specifically #246), we were on assert-plus@0.1, where assertplus.object(null) did not throw an exception. According to the assert-plus changelog, v0.2.0 changed that so that it does throw. I think that's the source of the breakage here, and we should be able to fix this by replacing the assert.object with assert.optionalObject.

By the way, this is easy to reproduce locally. I first set up a TCP server with nc(1):

$ nc -l 64000

Then I use mls, pointing it at the nc server:

$ MANTA_URL=http://localhost:64000 mls

At this point, the request headers show up in the "nc" terminal. Now hit CTRL-D or CTRL-C over there to kill "nc", and "mls" reports:

$ MANTA_URL=http://localhost:64000 mls
mls: AssertionError: undefined (object) is required
davepacheco commented 8 years ago

Note that the ECONNRESET is specious. Node assigns this error code to a "socket hang up" error in the HTTP module, which appears to mean any HTTP protocol-level error (e.g., reading end-of-stream without any response headers). There's no reason to believe that ECONNRESET was returned by a socket operation here.