Don't use `Kinetic.errors.XXX` for client API

[adrienverge]

Kinetic errors (such as Kinetic.errors.SUCCESS, Kinetic.errors.HMAC_FAILURE, Kinetic.errors.NOT_AUTHORIZED...) are special, protocol-defined values meant to be embedded in Kinetic messages. There are not meant to be returned to client code calling the library.

Some Kinetic lib functions (e.g. send and _parse) should be modified to return in a Node.js way (either throw exception or take a callback).

[koolfunky]

Ok I put this in my TODO

[koolfunky]

Something like that ?

    _parse(data, callback) {
        const version = data.readInt8(0);
        const pbMsgLen = data.readInt32BE(1);
        const chunkLen = data.readInt32BE(5);

        if (version !== getVersion()) {
            return callback(new Error('Version Failure'));
        }

        try {
            this._cmd = protoBuild.Message.decode(data.slice(9, 9 + pbMsgLen));
            this.setProtobuf(protoBuild.Command.decode(this._cmd.commandBytes));
        } catch (e) {
            if (e.decoded) {
                this.setProtobuf(e.decoded);
            } else {
                return callback(new Error('Error in decode' + e));
            }
        }
        this.setChunk(data.slice(pbMsgLen + 9, chunkLen + pbMsgLen + 9));

        if (this.getChunkSize() !== chunkLen) {
            return callback(new Error('data error - different len'));
        }
        if (this._cmd.authType === 1 &&
                !this.hmacIntegrity(this.getSlice(this._cmd.hmacAuth.hmac)))
            return callback(new Error('Hmac fail'));
        return callback(null);
    }

with a constructor like this :

    constructor(input, callback) {
        /*
         * From Kinetic official documentation, a Kinetic Protocol Data Unit
         * (PDU) is composed of:
         * - message -- a Protocol Buffer (protobuf) message, containing
         *              operation metadata & key-value metadata,
         * - chunk   -- the value to store/read.
         */
        this._message = undefined;
        this._chunk = undefined;

        if (input !== undefined) {
            if (!Buffer.isBuffer(input))
                return callback(new Error("input is not a buffer"));
            this._parse(input, (err) => {
                if (err){
                    return callback(new Error("could not parse input buffer (" + err + ")"));
                } else {
                    return callback(null, this);
                }
            });
        }
    }

[adrienverge]

Since both _parse and constructor are synchronous, I wouldn't use callbacks here.

Instead, they could throw exceptions, explicitely say it their jsdoc, and all code calling them should check for exceptions.

Example:

_parse(data) {
    if (version !== getVersion()) {
        const err = new Error('Version Failure');
        err.badVersion = true;
        throw err;
    }
}

Client code:

try {
    const pdu = new Kinetic.PDU(new Buffer("sdkfljsdf"));
} catch (e) {
    if (e.badVersion) {
        log.error("bad version!");
    }
}

[koolfunky]

Okok

And for send ? I tried something like that but i'm not really good on error handling, so I don't really know what is the best way

        try {
            if (this.getChunk() !== undefined)
                sock.write(Buffer.concat(
                    [pduHeader, this._message.toBuffer(), this.getChunk()]));
            else
                sock.write(Buffer.concat([pduHeader,
                    this._message.toBuffer()]));
        } catch (e) {
            return callback(e);
        }
        return callback(null);

I did this before, I just forgot to update my comment X)

[koolfunky]

I tried to do a better error throwing/handling on the branch dev/CLEANUP/error_return
There are some tests on the branch

[adrienverge]

Thanks @AntoninCoulibaly. The branch you created brings lots of enhancements, can you create a pull request for it? I have a few comments to make, but mostly 👍

[koolfunky]

Ok no problem I will

[Deleted user]

Closing, since this is not relevant in this repo anymore.