Upgrade node v14 to v16

[tangledbytes]

Explain the changes

This PR updates Node v14 to v16.14.0.

Node v16.14.0 was chosen despite v16.16.0 being available due to https://github.com/npm/cli/blob/4c945302fc2aa6854dc014fe31d6f5dfa96f7b52/lib/npm.js#L247 in nodejs v16.16.0 which made this version almost unusable in container environment without additional changes (fixed in later version of node v16).

Fixes for Node v16

1. Unhandled rejections are converted into uncaughtException1 in Node v16. This caused NooBaa to panic at many points. This was fixed by reverting to older behaviour by using --unhandled-rejections=warn. Node incorrectly throws ERR_UNHANDLED_REJECTION on error caught in Promise.catch nodejs/node#43326 was created in NodeJS regarding nuances recently introduced.

Other fixes

1. _flush() in ChunkFS called _flush_buffers() without awaiting it. This caused some times tests to fail non-deterministically (if the buffer is not empty by the last write). _flush_buffers() call is awaited now in the _flush() method.

Testing Instructions:

1. make test
2. make test-postgres
3. make tester; docker run --privileged --rm --name test1 noobaa-tester ./src/test/unit_tests/run_npm_test_on_test_container.sh -s sudo_index.js

• Doc added/updated
• Tests added

[liranmauda]

Hi @Utkarsh-pro
This PR is missing the package-lock in the new format.
Please create a PR that only updates the package-lock and not bumping anything.

[guymguym]

Hey @Utkarsh-pro
_flush() is not really an async function in nodejs streams, and the caller will not await for its promise, so it's a bit misleading. It passes a callback function instead - which we handle correctly inside _flush_buffers. The fact that we defined _flush() itself to be async was accidental. So if you get complaints on unhandled rejection of the promise returned by _flush_buffers() I would instead add a noop catch handler to it like _flush_buffers(callback).catch(noop) - you can see how we do it in other places -

noobaa-core/src/server/system_services/system_store.js

Line 374 in cfbf726

.catch(_.noop);

noobaa-core/src/rpc/ice.js

Line 1323 in 7a31c5d

self.defer.promise.catch(_.noop); // to ignore 'Unhandled rejection' printouts

noobaa-core/src/util/postgres_client.js

Line 1466 in db1bc28

seq.init_promise = seq._create(this.pool).catch(_.noop); // TODO what is best to do when init_collection fails here?

[tangledbytes]

@guymguym, actually that's true but for recent node versions. In node v16, although undocumented, it does calls then if a promise is returned. https://github.com/nodejs/node/blob/418ff70b810f0e7112d48baaa72932a56cfa213b/lib/internal/streams/transform.js#L131

Here's my understanding code, please correct me if this is wrong:

1. We have async _flush (accidental probably). This function always returns a promise.
2. Node sees that the returned value has .then attached to it.
3. As soon as the promise is resolved (which without await on our flush_buffer resolves almost immediately).
4. This leads to call this.push(null) in the node source.
5. Call to this.push(null) leads to emitting of readable event.
6. That leads to calling next which realises that oops, we have read everything.
7. Emits end event.
8. finished() already encountered the finish event by this time, only thing holding it back from getting resolved was the end event from the transform stream.
9. Above emit of end event caused the finished() to resolve.

And that's how I think we exit the finished before we intended. As mentioned, this causes the test to non-deterministic, sometimes promise resolves first (when read buffer indeed requires flushing) and sometimes callback is invoked first (desired).

[tangledbytes]

Here is a sample which won't work in node v16.14.0 (but would in recent and in v14):

const util = require("util");
const timeout = util.promisify(setTimeout);
const crypto = require("crypto");
const { Readable, Transform, pipeline, finished } = require("stream");
const pipelineAsync = util.promisify(pipeline);
const finishedAsync = util.promisify(finished);

async function P(...streams) {
  return pipelineAsync(...streams).then(() => {
    streams[streams.length - 1].resume();
  });
}

class TargetHash {
  constructor() {
    this.hash = crypto.createHash("md5");
  }

  digest() {
    return this.hash.digest("hex");
  }

  async writev(_config, buffers) {
    await timeout(500);
    for (const buf of buffers) this.hash.update(buf);
  }
}

class T extends Transform {
  constructor({ targetHash }) {
    super();
    this.th = targetHash;
  }

  async _transform(chunk, encoding, callback) {
    await this.th.writev({}, [chunk]);
    callback();
  }

  async _flush(callback) {
    console.log("flush start");
    this._flush_work(callback);
    console.log("flush end");
  }

  async _flush_work(cb) {
    console.log("flush_work start");
    await this.th.writev({}, [Buffer.from("EOF")]);
    console.log("flush_work end");

    console.log("flush_work cb start");
    const res = cb();
    console.log("flush_work cb end");
    return res;
  }
}

async function main() {
  const th = new TargetHash();
  const t = new T(
    { targetHash: th },
  );
  const r = Readable.from(
    (async function* () {
      for (let i = 0; i < 1e1; i++) {
        yield i.toString();
      }
    })()
  );

  console.log("pipeline start");
  await P(r, t);
  await finishedAsync(t);
  console.log("DIGEST:", th.digest());
  console.log("pipeline finished");
}

main();

[guymguym]

@Utkarsh-pro Did you consider setting it in the NODE_OPTIONS env instead of adding to all calls?
See https://nodejs.org/docs/latest-v16.x/api/cli.html#node_optionsoptions
But if we are going to clean up these warnings soon, as we should, then it doesn't matter anyway.

[tangledbytes]

@guymguym, actually I did open up an issue for cleaning this up here: #7083. But as I mentioned in the issue, I think we would need a tiny bit more help from the runtime as right now it doesn't event shows that the uncaughtException was rooted from unhandledRejection and has no traces of it in the stack trace either. That's why resorted to this.

I think I completely missed considering the node options though, thank you!

[liranmauda]

@Utkarsh-pro also it seems that --unhandled-rejections=warn is missing here:

noobaa-core/src/deploy/NVA_build/noobaa_init.sh

Line 190 in cfbf726

run_internal_process node ./src/s3/s3rver_starter.js