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

Update dependencies for a Node v6 age #246

Closed
chorrell opened this issue Jan 4, 2016 · 27 comments
Closed

Update dependencies for a Node v6 age #246

chorrell opened this issue Jan 4, 2016 · 27 comments
Assignees

Comments

@chorrell
Copy link

chorrell commented Jan 4, 2016

When installing manta under Node.js v5.3.0 and 3.3.12, there are several errors (non-fatal I think) when installing dtrace-provider.

For example:

gyp WARN EACCES user "root" does not have permission to access the dev dir "/Users/christopher/.node-gyp/5.3.0"
gyp WARN EACCES attempting to reinstall using temporary dev dir "/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp"
  ACTION binding_gyp_libusdt_target_build_libusdt .
Building libusdt for x86_64
rm -f *.gch
rm -f *.o
rm -f libusdt.a
rm -f test_usdt
rm -f test_usdt32
rm -f test_usdt64
rm -f test_mem_usage
gcc -O2 -arch x86_64   -c -o usdt.o usdt.c
usdt.c:212:9: warning: implicit declaration of function 'usdt_dof_section_free' is invalid
      in C99 [-Wimplicit-function-declaration]
        usdt_dof_section_free(&strtab);
        ^
usdt.c:238:9: warning: implicit declaration of function 'usdt_dof_file_free' is invalid in
      C99 [-Wimplicit-function-declaration]
        usdt_dof_file_free(provider->file);
        ^
2 warnings generated.
gcc -O2 -arch x86_64   -c -o usdt_dof_file.o usdt_dof_file.c
gcc -arch x86_64 -o usdt_tracepoints.o -c usdt_tracepoints_x86_64.s
gcc -O2 -arch x86_64   -c -o usdt_probe.o usdt_probe.c
gcc -O2 -arch x86_64   -c -o usdt_dof.o usdt_dof.c
gcc -O2 -arch x86_64   -c -o usdt_dof_sections.o usdt_dof_sections.c
rm -f libusdt.a
ar cru libusdt.a usdt.o usdt_dof_file.o usdt_tracepoints.o usdt_probe.o usdt_dof.o usdt_dof_sections.o 
ranlib libusdt.a
  TOUCH Release/obj.target/libusdt.stamp
  CXX(target) Release/obj.target/DTraceProviderBindings/dtrace_provider.o
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:69:44: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> New(const v8::Arguments& args);
                                           ^~~~~~~~~~~~~
                                           v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:70:45: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> Fire(const v8::Arguments& args);
                                            ^~~~~~~~~~~~~
                                            v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:87:44: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> New(const v8::Arguments& args);
                                           ^~~~~~~~~~~~~
                                           v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:88:49: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> AddProbe(const v8::Arguments& args);
                                                ^~~~~~~~~~~~~
                                                v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:89:52: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> RemoveProbe(const v8::Arguments& args);
                                                   ^~~~~~~~~~~~~
                                                   v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:90:47: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> Enable(const v8::Arguments& args);
                                              ^~~~~~~~~~~~~
                                              v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:91:48: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> Disable(const v8::Arguments& args);
                                               ^~~~~~~~~~~~~
                                               v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
In file included from ../dtrace_provider.cc:1:
../dtrace_provider.h:92:45: error: no type named 'Arguments' in namespace 'v8'; did you mean
      'v8::internal::Arguments'?
    static v8::Handle<v8::Value> Fire(const v8::Arguments& args);
                                            ^~~~~~~~~~~~~
                                            v8::internal::Arguments
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:139:7: note: 
      'v8::internal::Arguments' declared here
class Arguments;
      ^
../dtrace_provider.cc:23:17: error: calling a protected constructor of class
      'v8::HandleScope'
    HandleScope scope;
                ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:889:13: note: 
      declared protected here
  V8_INLINE HandleScope() {}
            ^
../dtrace_provider.cc:25:55: error: cannot initialize a parameter of type 'v8::Isolate *'
      with an lvalue of type 'v8::Handle<v8::Value> (const v8::internal::Arguments &)'
    Local<FunctionTemplate> t = FunctionTemplate::New(DTraceProvider::New);
                                                      ^~~~~~~~~~~~~~~~~~~
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:4349:16: note: 
      passing argument to parameter 'isolate' here
      Isolate* isolate, FunctionCallback callback = 0,
               ^
../dtrace_provider.cc:26:58: error: 'New' is a private member of
      'v8::PersistentBase<v8::FunctionTemplate>'
    constructor_template = Persistent<FunctionTemplate>::New(t);
                                                         ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:643:23: note: 
      declared private here
  V8_INLINE static T* New(Isolate* isolate, T* that);
                      ^
../dtrace_provider.cc:26:28: error: too few arguments to function call, expected 2, have 1;
      did you mean 'DTraceProbe::New'?
    constructor_template = Persistent<FunctionTemplate>::New(t);
                           ^~~~~~~~~~
                           DTraceProbe::New
../dtrace_provider.h:69:34: note: 'DTraceProbe::New' declared here
    static v8::Handle<v8::Value> New(const v8::Arguments& args);
                                 ^
../dtrace_provider.cc:27:25: error: member reference type 'Persistent<v8::FunctionTemplate>'
      is not a pointer; maybe you meant to use '.'?
    constructor_template->InstanceTemplate()->SetInternalFieldCount(1);
    ~~~~~~~~~~~~~~~~~~~~^~
                        .
../dtrace_provider.cc:27:27: error: no member named 'InstanceTemplate' in
      'v8::Persistent<v8::FunctionTemplate,
      v8::NonCopyablePersistentTraits<v8::FunctionTemplate> >'
    constructor_template->InstanceTemplate()->SetInternalFieldCount(1);
    ~~~~~~~~~~~~~~~~~~~~  ^
../dtrace_provider.cc:28:25: error: member reference type 'Persistent<v8::FunctionTemplate>'
      is not a pointer; maybe you meant to use '.'?
    constructor_template->SetClassName(String::NewSymbol("DTraceProvider"));
    ~~~~~~~~~~~~~~~~~~~~^~
                        .
../dtrace_provider.cc:28:27: error: no member named 'SetClassName' in
      'v8::Persistent<v8::FunctionTemplate,
      v8::NonCopyablePersistentTraits<v8::FunctionTemplate> >'
    constructor_template->SetClassName(String::NewSymbol("DTraceProvider"));
    ~~~~~~~~~~~~~~~~~~~~  ^
../dtrace_provider.cc:28:48: error: no member named 'NewSymbol' in 'v8::String'
    constructor_template->SetClassName(String::NewSymbol("DTraceProvider"));
                                       ~~~~~~~~^
../dtrace_provider.cc:30:31: error: no viable conversion from
      'Persistent<v8::FunctionTemplate>' to 'v8::Local<v8::FunctionTemplate>'
    NODE_SET_PROTOTYPE_METHOD(constructor_template, "addProbe", DTraceProvider::AddProbe);
                              ^~~~~~~~~~~~~~~~~~~~
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:210:7: note: 
      candidate constructor (the implicit copy constructor) not viable: no known conversion
      from 'Persistent<v8::FunctionTemplate>' to 'const v8::Local<v8::FunctionTemplate> &'
      for 1st argument
class Local {
      ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:210:7: note: 
      candidate constructor (the implicit move constructor) not viable: no known conversion
      from 'Persistent<v8::FunctionTemplate>' to 'v8::Local<v8::FunctionTemplate> &&' for
      1st argument
class Local {
      ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:214:13: note: 
      candidate template ignored: could not match 'Local' against 'Persistent'
  V8_INLINE Local(Local<S> that)
            ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:326:13: note: 
      candidate template ignored: could not match 'S *' against
      'Persistent<v8::FunctionTemplate>'
  V8_INLINE Local(S* that)
            ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/node.h:252:71: note: 
      passing argument to parameter 'recv' here
inline void NODE_SET_PROTOTYPE_METHOD(v8::Local<v8::FunctionTemplate> recv,
                                                                      ^
../dtrace_provider.cc:31:31: error: no viable conversion from
      'Persistent<v8::FunctionTemplate>' to 'v8::Local<v8::FunctionTemplate>'
    NODE_SET_PROTOTYPE_METHOD(constructor_template, "removeProbe", DTraceProvider::R...
                              ^~~~~~~~~~~~~~~~~~~~
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:210:7: note: 
      candidate constructor (the implicit copy constructor) not viable: no known conversion
      from 'Persistent<v8::FunctionTemplate>' to 'const v8::Local<v8::FunctionTemplate> &'
      for 1st argument
class Local {
      ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:210:7: note: 
      candidate constructor (the implicit move constructor) not viable: no known conversion
      from 'Persistent<v8::FunctionTemplate>' to 'v8::Local<v8::FunctionTemplate> &&' for
      1st argument
class Local {
      ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:214:13: note: 
      candidate template ignored: could not match 'Local' against 'Persistent'
  V8_INLINE Local(Local<S> that)
            ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/v8.h:326:13: note: 
      candidate template ignored: could not match 'S *' against
      'Persistent<v8::FunctionTemplate>'
  V8_INLINE Local(S* that)
            ^
/usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider/.node-gyp/5.3.0/include/node/node.h:252:71: note: 
      passing argument to parameter 'recv' here
inline void NODE_SET_PROTOTYPE_METHOD(v8::Local<v8::FunctionTemplate> recv,
                                                                      ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make: *** [Release/obj.target/DTraceProviderBindings/dtrace_provider.o] Error 1
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:270:23)
gyp ERR! stack     at emitTwo (events.js:87:13)
gyp ERR! stack     at ChildProcess.emit (events.js:172:7)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)
gyp ERR! System Darwin 15.2.0
gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /usr/local/lib/node_modules/manta/node_modules/restify/node_modules/dtrace-provider
gyp ERR! node -v v5.3.0
gyp ERR! node-gyp -v v3.0.3
gyp ERR! not ok 
npm WARN install:dtrace-provider@0.2.8 dtrace-provider@0.2.8 install: `node-gyp rebuild`
npm WARN install:dtrace-provider@0.2.8 Exit status 1

Updating bunyan, restify and watershed to the latest versions in package.json seems to resolve this:

"bunyan": "1.5.1"
"restify": "4.0.3"
"watershed": "0.3.1"

Full package.json:

{
    "name": "manta",
    "author": "Joyent (joyent.com)",
    "contributors": [
        "Dave Eddy"
    ],
    "description": "Manta Client API",
    "homepage": "http://apidocs.joyent.com/manta",
    "repository": {
        "type": "git",
        "url": "git://github.com/joyent/node-manta.git"
    },
    "version": "2.0.5",
    "main": "./lib/index.js",
    "dependencies": {
        "assert-plus": "0.1.5",
        "backoff": "2.3.0",
        "bunyan": "1.5.1",
        "clone": "0.1.11",
        "cmdln": "1.3.1",
        "dashdash": "1.3.2",
        "extsprintf": "1.0.2",
        "hogan.js": "2.0.0",
        "jsprim": "0.8.0",
        "lomstream": "1.1.0",
        "lstream": "0.0.3",
        "mime": "1.2.11",
        "node-uuid": "1.4.1",
        "moment": "2.4.0",
        "once": "1.3.0",
        "path-platform": "0.0.1",
        "progbar": "1.0.0",
        "readable-stream": "1.1.9",
        "restify": "4.0.3",
        "showdown": "0.3.1",
        "smartdc-auth": "2.3.1",
        "tar": "0.1.18",
        "vasync": "1.4.0",
        "verror": "1.3.6",
        "watershed": "0.3.1"
    },
    "devDependencies": {
        "nodeunit": "0.9.0"
    },
    "directories": {
        "bin": "./bin",
        "lib": "./lib",
        "man": "./man/man1"
    },
    "engines": {
        "node": ">=0.8.14"
    },
    "scripts": {
        "test": "./node_modules/.bin/nodeunit test/*.test.js"
    }
}

Not sure what the version should be bumped too. 2.1.0?

@davepacheco
Copy link
Contributor

2.1.0 is fine for this change.

This change bumps bunyan across one major version and restify across two major versions. That's fine, but it would be good to review the breaking changes in those APIs to make sure we won't be broken by this.

This overlaps with #245, where I raised the same issue.

@chorrell
Copy link
Author

chorrell commented Jan 4, 2016

Yeah, the version bump for restify makes me nervous. I did some quick testing using more recent 2.8.X and 3.x.x versions, but none of those seemed to resolve the dtrace-provider issue.

@davepacheco
Copy link
Contributor

We should upgrade to latest if we can -- it's just that we should look at what the breaking changes are and make sure they won't affect us (or that we fix the code).

@chorrell
Copy link
Author

chorrell commented Jan 4, 2016

@chorrell
Copy link
Author

chorrell commented Jan 4, 2016

For bunyan, the notable breaking changes are in 1.0.0:

  • "Backward incompatible change to -c CODE improving performance by over 10x (good!), with a backward incompatible change to semantics (unfortunate), and adding some sugar (good!)". --> 'this' is now required to access the object fields
  • "Backward incompatible. Drop dtrace-provider even from optionalDependencies". It was added back in 1.2.0, then updated to 0.6 in the 1.5.0 release

@trentm
Copy link
Contributor

trentm commented Feb 18, 2016

Here is my diff that gets these and also updates tar and showdown to avoid the "deprecated" warnings from npm:

diff --git a/lib/jobshare.js b/lib/jobshare.js
index 840bb11..56b7278 100644
--- a/lib/jobshare.js
+++ b/lib/jobshare.js
@@ -258,7 +258,7 @@ function jStageRender(j, cb)

     params['manta_url'] = process.env['MANTA_URL'];

-    converter = new showdown.converter();
+    converter = new showdown.Converter();
     if (j.j_data.hasOwnProperty('readme'))
         params['readme'] = converter.makeHtml(j.j_data['readme']['contents']);

diff --git a/package.json b/package.json
index 4fd5889..cdaa95b 100644
--- a/package.json
+++ b/package.json
@@ -15,7 +15,7 @@
     "dependencies": {
         "assert-plus": "0.1.5",
         "backoff": "2.3.0",
-        "bunyan": "0.22.1",
+        "bunyan": "1.6.0",
         "clone": "0.1.11",
         "cmdln": "1.3.1",
         "dashdash": "1.3.2",
@@ -31,13 +31,13 @@
         "path-platform": "0.0.1",
         "progbar": "1.0.0",
         "readable-stream": "1.1.9",
-        "restify": "2.8.1",
-        "showdown": "0.3.1",
+        "restify": "4.0.4",
+        "showdown": "1.3.0",
         "smartdc-auth": "2.3.1",
-        "tar": "0.1.18",
+        "tar": "2.2.1",
         "vasync": "1.4.0",
         "verror": "1.3.6",
-        "watershed": "0.3.0"
+        "watershed": "0.3.1"
     },
     "devDependencies": {
         "nodeunit": "0.9.0"

However I get one test failure:

client.test
✔ mkdir
✔ mkdir (sub)
✔ put
✔ #231: put (special characters)
✔ #231: ls (special characters)
✔ #231: get (special characters)
✔ #231: rm (special characters)
✖ chattr

InvalidUpdateError: overwrite of "content-length" forbidden
    at ClientRequest.onResponse (/Users/trentm/joy/node-manta/node_modules/restify/lib/clients/http_client.js:169:26)
    at ClientRequest.g (events.js:273:16)
    at emitOne (events.js:90:13)
    at ClientRequest.emit (events.js:182:7)
    at HTTPParser.parserOnIncomingClient (_http_client.js:425:21)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:89:23)
    at TLSSocket.socketOnData (_http_client.js:315:20)
    at emitOne (events.js:90:13)
    at TLSSocket.emit (events.js:182:7)
    at readableAddChunk (_stream_readable.js:147:16)
    at TLSSocket.Readable.push (_stream_readable.js:111:10)
    at TLSWrap.onread (net.js:525:20)

AssertionError
    at Object.equal (/Users/trentm/joy/node-manta/node_modules/nodeunit/lib/types.js:83:39)
    at onInfo (/Users/trentm/joy/node-manta/test/client.test.js:242:23)
    at /Users/trentm/joy/node-manta/lib/client.js:1095:21
    at IncomingMessage.onEnd (/Users/trentm/joy/node-manta/lib/client.js:468:13)
    at IncomingMessage.g (events.js:273:16)
    at emitNone (events.js:85:20)
    at IncomingMessage.emit (events.js:179:7)
    at endReadableNT (_stream_readable.js:906:12)
    at nextTickCallbackWith2Args (node.js:475:9)
    at process._tickCallback (node.js:389:17)

✔ put (zero byte streaming)
✔ put without mkdirp
...

which I haven't dug into.

@trentm
Copy link
Contributor

trentm commented Feb 18, 2016

More details on that test failure when running with restify 4.0.4 and node 5. From the trace log there is this:

[2016-02-18T07:19:41.822Z] DEBUG: /Users/trentm/joy/node-manta/node_modules/.bin/nodeunit/MantaClient/74637 on danger0.local (/Users/trentm/joy/node-manta/lib/client.js:708 in chattr): chattr: entered (req_id=08533ced-3bfe-47da-a627-ddb6d07dcd0d, id=08533ced-3bfe-47da-a627-ddb6d07dcd0d, query={})
    path: /trent.mick/stor/6b9105ef-5222-4914-909f-4630761c9648/d0991679-ea18-4c5f-9d38-cdc18087f414?metadata=true
    --
    headers: {
      "m-foo": "bar",
      "accept": "*/*",
      "x-request-id": "08533ced-3bfe-47da-a627-ddb6d07dcd0d"
    }
[2016-02-18T07:19:41.920Z] DEBUG: /Users/trentm/joy/node-manta/node_modules/.bin/nodeunit/MantaClient/74637 on danger0.local (/Users/trentm/joy/node-manta/lib/client.js:332 in onReadDone): chattr: error (req_id=08533ced-3bfe-47da-a627-ddb6d07dcd0d, err.code=InvalidUpdate)
    InvalidUpdateError: overwrite of "content-length" forbidden
        at ClientRequest.onResponse (/Users/trentm/joy/node-manta/node_modules/restify/lib/clients/http_client.js:169:26)
        at ClientRequest.g (events.js:273:16)
        at emitOne (events.js:90:13)
        at ClientRequest.emit (events.js:182:7)
        at HTTPParser.parserOnIncomingClient (_http_client.js:425:21)
        at HTTPParser.parserOnHeadersComplete (_http_common.js:89:23)
        at TLSSocket.socketOnData (_http_client.js:315:20)
        at emitOne (events.js:90:13)
        at TLSSocket.emit (events.js:182:7)
        at readableAddChunk (_stream_readable.js:147:16)
        at TLSSocket.Readable.push (_stream_readable.js:111:10)
        at TLSWrap.onread (net.js:525:20)
    --
    path: /trent.mick/stor/6b9105ef-5222-4914-909f-4630761c9648/d0991679-ea18-4c5f-9d38-cdc18087f414

The error is unexpected. When running with restify 4.0.4 and node 0.10.42 I get:

$ make test
...
[2016-02-18T07:22:58.054Z] DEBUG: /Users/trentm/joy/node-manta/node_modules/.bin/nodeunit/MantaClient/76676 on danger0.local (/Users/trentm/joy/node-manta/lib/client.js:708 in chattr): chattr: entered (req_id=9902f692-ab51-499f-91e6-f31d83f317d7, id=9902f692-ab51-499f-91e6-f31d83f317d7, query={})
    path: /trent.mick/stor/f1d0210f-9f81-4f46-a313-31d674ccd6b1/39b40146-671d-4b1c-8fe8-5438445a75e3?metadata=true
    --
    headers: {
      "m-foo": "bar",
      "accept": "*/*",
      "x-request-id": "9902f692-ab51-499f-91e6-f31d83f317d7"
    }
[2016-02-18T07:22:58.160Z] DEBUG: /Users/trentm/joy/node-manta/node_modules/.bin/nodeunit/MantaClient/76676 on danger0.local (/Users/trentm/joy/node-manta/lib/client.js:337 in onEnd): chattr: done (req_id=9902f692-ab51-499f-91e6-f31d83f317d7)
    path: /trent.mick/stor/f1d0210f-9f81-4f46-a313-31d674ccd6b1/39b40146-671d-4b1c-8fe8-5438445a75e3
XXX err1 null
✔ chattr

So this is a diff in what the node HTTP client sends between node 0.10 and node 5?

@trentm
Copy link
Contributor

trentm commented Feb 18, 2016

Would be interesting to see what the Manta server sees for those req_ids.

@davepacheco
Copy link
Contributor

The InvalidUpdateError appears to be a result of nodejs/node#1062, which explains why we saw it only after Node 0.12. We'll probably need a server-side change for this. This is tracked by internal ticket MANTA-2846.

@BobDickinson
Copy link

Got the same installing 2.0.7 on Node 5.8.0. Seems to work fine. Pretty disconcerting for a new user to see this kind of install output.

This was referenced Jun 13, 2016
@papertigers
Copy link

bump
@davepacheco looks like MANTA-2846 has been fixed.
@trentm was that the only broken test you saw? I will try and rerun the tests myself.

@papertigers
Copy link

papertigers commented Jun 22, 2016

The patch:

diff --git a/lib/jobshare.js b/lib/jobshare.js
index 840bb11..56b7278 100644
--- a/lib/jobshare.js
+++ b/lib/jobshare.js
@@ -258,7 +258,7 @@ function jStageRender(j, cb)

     params['manta_url'] = process.env['MANTA_URL'];

-    converter = new showdown.converter();
+    converter = new showdown.Converter();
     if (j.j_data.hasOwnProperty('readme'))
         params['readme'] = converter.makeHtml(j.j_data['readme']['contents']);

diff --git a/package.json b/package.json
index 23b44d0..fc21a5d 100644
--- a/package.json
+++ b/package.json
@@ -15,7 +15,7 @@
     "dependencies": {
         "assert-plus": "0.1.5",
         "backoff": "2.3.0",
-        "bunyan": "0.22.1",
+        "bunyan": "1.6.0",
         "clone": "0.1.11",
         "cmdln": "1.3.1",
         "dashdash": "1.3.2",
@@ -32,12 +32,12 @@
         "progbar": "1.0.0",
         "readable-stream": "1.1.9",
         "restify": "2.8.1",
-        "showdown": "0.3.1",
+        "showdown": "1.3.0",
         "ssh-agent": "0.2.2",
-        "tar": "0.1.18",
+        "tar": "2.2.1",
         "vasync": "1.4.0",
         "verror": "1.3.6",
-        "watershed": "0.3.0"
+        "watershed": "0.3.1"
     },
     "devDependencies": {
         "nodeunit": "0.8.1"

The tests

client.test.js
✔ mkdir
✔ mkdir (sub)
✔ put
✔ chattr
✔ put (zero byte streaming)
✔ put without mkdirp
✔ put with mkdirp
✔ streams
✔ put MD5 mismatch
✔ GH-72 content-length: undefined
✔ ls
✔ ln
✔ info (link)
✔ ftw
✔ create job (simple grep)
✔ get job
✔ add input keys
✔ get job input
✔ end job
✔ wait for job
✔ get job output
✔ create and cancel job
✔ unlink object
✔ unlink link
✔ rmr
✔ mkdirp/rmr
✔ GH-196 getPath ~~/
✔ #180: Invalid key results in no client error

OK: 84 assertions (14694ms)

@davepacheco
Copy link
Contributor

davepacheco commented Jun 22, 2016

Okay, so there are three different sets of updates here:

Presumably we're going to want to stick with Trent's patch because we want to update restify to latest, too? Or maybe we should change that to use the newer restify-clients package. It would be great if we could also update graceful-fs, but that could be a follow-on change.

As I understand it, these are the remaining open blockers:

  • restify has jumped two major versions. Do any of the breaking changes affect node-manta?
  • showdown has jumped one major version. Do the breaking changes there affect node-manta?
  • tar has jumped two major versions. Do any of the breaking changes affect node-manta?
  • While MANTA-2846 has been resolved, the underlying issue is still there. (The ticket ended up covering two issues, and only one of them has been fixed.) Essentially, a change to Node made in the io.js era exposed a server issue, and I don't think we can reasonably say we support Node > 0.12 until we fix the server issue as well. (We can still update the dependencies, and consumers likely won't notice this if they don't use mchmod/mchattr.)

Obviously, the biggest thing is verifying the major version bumps. This isn't as simple as just running the tests, because major version bumps can involve arbitrary semantic changes. Someone just needs to go through the changelogs for those repos, understand what was broken, and then see if any node-manta code is affected.

@davepacheco
Copy link
Contributor

I've filed https://smartos.org/bugview/MANTA-2929 for the server-side issue related to InvalidUpdateError.

@trentm
Copy link
Contributor

trentm commented Jun 22, 2016

On the restify update:

-        "restify": "2.8.1",
+        "restify": "4.0.4",

tl;dr: I suspect we'll be fine jumping to restify-clients 1.x. I'll get a PR
for this and start testing against versions of node.

restify 2 -> 3: restify/node-restify#753

https://github.com/restify/node-restify/commit/ab1a7522af51a4337468f06cf4b929b05fba458f

- #753 **BREAKING** Include `err` parameter for all \*Error events:
  Error events will all have the signature `function (req, res, err, cb)` to
  become consistent with the handling functionality introduced in 2.8.5.
  Error handlers using the `function (req, res, cb)` signature must be updated.

This is server-only so won't affect node-manta usage.

restify-clients was split out to 1.0.0 with this timeline:

{ modified: '2016-03-16T00:40:59.873Z',
  created: '2015-06-30T21:26:25.803Z',
  '1.0.0': '2015-06-30T21:26:25.803Z',
  '1.1.0': '2015-09-09T15:06:03.633Z',
  '1.1.1': '2015-10-14T02:09:47.320Z',
  '1.1.2': '2016-01-08T18:49:34.295Z',
  '1.2.0': '2016-03-02T17:26:44.002Z',
  '1.2.1': '2016-03-16T00:40:59.873Z' }

That's basically when the restify 4.x release happened:

'3.0.3': '2015-05-19T22:18:33.040Z',
'4.0.0': '2015-08-14T01:17:04.221Z',

It isn't clear to me what in the following triggers a 4.x for restify (server):

## 4.0.0
- #877 content-type can be case-insensitive. Yunong Xiao
- #856 update various dependencies. Alex Liu
- #851 fix formatters such that they always return cb. Yunong Xiao
- #847 fix body parser race condition. Yunong Xiao
- #842 add `req.matchedVersion()` Nathan Peck, Micah Ransdell
- #840 Fix issue with server toString Method. OiNutter, Micah Ransdell
- #836 Add JSDoc comments. Alex Liu
- #835 Update static.js to allow for serving static files that do not use the route as a path. Wavewash, Micah Ransdell
- #831 Support hash option to Formidable for multipart file uploads. blakevanian, ManRueda
- #832 Updated dtrace-provider. yads
- #812 add query parameters to auditlogger. Alex Liu
- #800 Allow 0, false, and null as json body. Alex Dobeck
- #771 q-value choice on wildcards ignores default q-value of 1. Kevin Peno
- #822 Allow optional headers to be added as properties to bunyan logs. Michael Paulson.
- #824 Don't include large coverage files in published packages. Trent Mick
- #819 Add a feature to allow the expiration of old unprocessed requests. Michael Paulson
- #803 Add redirect support to Response. Alex Liu
- #686 `res.send` can't send 0, false and null. Alex Dobeck

Notes:

  • this is a server API change: "#851 fix formatters such that they always
    return cb. Yunong Xiao"
  • ... review of the rest of the commits shows basically no changes to
    the clients area. The only clients change of late at all is addition for
    NO_PROXY handling.

@trentm
Copy link
Contributor

trentm commented Jun 22, 2016

The "InvalidUpdateError" issue has been moved to this ticket: MANTA-2929 (https://smartos.org/bugview/MANTA-2929).

@trentm trentm self-assigned this Jun 23, 2016
@trentm trentm changed the title Update bunyan, restify and watershed dependencies Update dependencies for a Node v6 age Jun 23, 2016
trentm added a commit that referenced this issue Jun 23, 2016
MANTA-2929: InvalidUpdateError on mchmod from clients after Node 0.12

- 'make testall' to somewhat easily test against all supported node
  versions
- Convert to using restify-clients instead of restify. This drops
  having the "bin/m*" tools set "serializers" in they logger because
  that gets added via "log.child" in "createBinClient".
- Add "license" field to package.json to avoid 'npm install' warning.
- Added "node-manta-test-" prefix to the dir created in Manta when
  running the test suite. This helps for manual cleaning up after
  failed test runs.
- Fix two places where 'make test' would hang while a Manta client had
  an open connection (the "agent:false" additions in client.test.js).
- MANTA-2929: ensure no implicit content-length by forcing chunked
  transfer-encoding for PutMetadata requests.
@trentm
Copy link
Contributor

trentm commented Jun 23, 2016

Doing work for this in the "nouveau" branch. Some notes on top-level deps that I won't be updating in this go round:

$ npm_config_depth=0 npm outdated
Package          Current  Wanted   Latest  Location
clone             0.1.11  0.1.11    1.0.2  clone
backoff            2.3.0   2.3.0    2.5.0  backoff
hogan.js           2.0.0   2.0.0    3.0.2  hogan.js
cmdln              1.3.1   1.3.1    4.1.1  cmdln
dashdash           1.3.2   1.3.2   1.14.0  dashdash
mime              1.2.11  1.2.11    1.3.4  mime
path-platform      0.0.1   0.0.1  0.11.15  path-platform
readable-stream    1.1.9   1.1.9    2.1.4  readable-stream


mime              1.2.11  1.2.11    1.3.4  mime
    1.3 moved from including the mime db itself to using separate mime-db
    module. Do this another time.

backoff            2.3.0   2.3.0    2.5.0  backoff
    Julien mentioned there might be something to watch for here. I haven't
    looked.

hogan.js           2.0.0   2.0.0    3.0.2  hogan.js
    meh, do later

clone             0.1.11  0.1.11    1.0.2  clone
    meh, do later

path-platform      0.0.1   0.0.1  0.11.15  path-platform
    SHould be safe. Ver bump is to match core 'path' added for node.js 0.11.15.

readable-stream    1.1.9   1.1.9    2.1.4  readable-stream
    todo later, this could be tricky

cmdln              1.3.1   1.3.1    4.1.1  cmdln
dashdash           1.3.2   1.3.2   1.14.0  dashdash
    dashdash 1.13.0 has this:
        [pull #16] Change the positiveInteger type to NOT accept zero (0). For
        those who might need the old behaviour, see
        "examples/custom-option-intGteZero.js". (By Dave Pacheco.)
    there are uses of that (a lot) in bin/m*
        -> XXX'd the two that matter
    separate ticket for these on me

@BobDickinson
Copy link

Just my $0.02, but as long as it installs clean, I'll be happy.

@trentm
Copy link
Contributor

trentm commented Jun 23, 2016

@BobDickinson Yup, hoping to have a commit for all this by tomorrow. Working on improving some testing of the various node versions as well.

@trentm
Copy link
Contributor

trentm commented Jun 23, 2016

On the node-tar major update:

-        "tar": "0.1.18",
+        "tar": "2.2.1",

https://github.com/npm/node-tar

1 -> 2 commit is because of this one I believe:

    commit a5337a6cd58a2d800fc03b3781a25751cf459f28
    Author: Forrest L Norvell <forrest@npmjs.com>
    Date:   Thu Mar 26 18:42:53 2015 -0700

        extract: normalize symbolic links outside root

0 -> 1 commit is because of this one I believe:

    commit 4dbe9a91a12ad7d66a8df197d4cebf7d7e7c775f
    Author: isaacs <i@izs.me>
    Date:   Thu Jul 31 15:29:31 2014 -0700

        fstream@1

    diff --git a/package.json b/package.json
    index 2656444..c2e370f 100644
    --- a/package.json
    +++ b/package.json
    @@ -13,7 +13,7 @@
       },
       "dependencies": {
         "block-stream": "*",
    -    "fstream": "~0.1.28",
    +    "fstream": "^1.0.0",
         "inherits": "2"
       },
       "devDependencies": {

fstream 0 -> 1: Sigh. The only thing between fstream@0.1.30 and fstream@1.0.0
is this:

    commit 4c758ec40d95ebe2870acd1ef65dd5b403f17e5e
    Author: isaacs <i@izs.me>
    Date:   Thu Jul 31 15:28:07 2014 -0700

        graceful-fs@latest

    diff --git a/package.json b/package.json
    index a1b8cca..2c69a9d 100644
    --- a/package.json
    +++ b/package.json
    @@ -12,7 +12,7 @@
         "node": ">=0.6"
       },
       "dependencies": {
    -    "graceful-fs": "~3.0.2",
    +    "graceful-fs": "^3.0.2",
         "inherits": "~2.0.0",
         "mkdirp": "^0.5.0",
         "rimraf": "2"

I don't anticipate compat issues from this one.

Mike is adding some muntar test cases to help shore up sanity testing there.

@trentm
Copy link
Contributor

trentm commented Jun 23, 2016

On showdown update:

-        "showdown": "0.3.1",
+        "showdown": "1.3.0",

commit eae5f0e01fd086cfee87de5a547fcd057f59a460 (Major code refactoring)
Looks like just a refactoring of the exported names to use capitals per:
https://github.com/showdownjs/showdown/blob/master/CHANGELOG.md#breaking-changes-1

@trentm
Copy link
Contributor

trentm commented Jun 23, 2016

All, some sanity usage of the nouveau branch would be helpful. You can install it globally via: npm install -g git://github.com/joyent/node-manta.git#nouveau

@trentm
Copy link
Contributor

trentm commented Jun 23, 2016

testing 'mjob share' (user of showdown, etc.): Hard to look at specific issues the update may have caused, but FWIW, 'mjob share ...' "worked" for a ~100 of my jobs that I tried. That doesn't mean a lot tho.

@bahamat
Copy link
Member

bahamat commented Jun 25, 2016

If I'm testing nouveau, should I also be using node 6? I'm currently mostly on 4.x.

@trentm
Copy link
Contributor

trentm commented Jun 25, 2016

Any node version >=0.10. The idea is that it works with all those versions. Node 4 is helpful testing.

--Trent

On Jun 24, 2016, at 5:08 PM, Brian Bennett notifications@github.com wrote:

If I'm testing nouveau, should I also be using node 6? I'm currently mostly on 4.x.


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.

@BobDickinson
Copy link

We admittedly do not push Manta super hard, but it installed clean and ran clean in our tests on Node versions 4.3.2, 5.8.0, and 6.0.0.

trentm added a commit that referenced this issue Jun 29, 2016
MANTA-2937: mchmod client-side workaround for MANTA-2929 InvalidUpdateError

Also:
- 'make testall' to somewhat easily test against all supported node
  versions
- "tools/test-in-smartos-container.sh" to use the `triton` CLI to
  create a test smartos container and run the test suite in a number of
  node versions.
- Convert to using restify-clients instead of restify. This drops
  having the "bin/m*" tools set "serializers" in they logger because
  that gets added via "log.child" in "createBinClient".
- Add "license" field to package.json to avoid 'npm install' warning.
- Added "node-manta-test-" prefix to the dir created in Manta when
  running the test suite. This helps for manual cleaning up after
  failed test runs.
- Fix two places where 'make test' would hang while a Manta client had
  an open connection (the "agent:false" additions in client.test.js).

Reviewed by: Dave Pacheco <dap@joyent.com>
Parts reviewed by: Joshua M. Clulow <jmc@joyent.com>
@trentm
Copy link
Contributor

trentm commented Jun 29, 2016

manta@3.0.0 published now.

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

No branches or pull requests

6 participants