A lot of changes, mainly new README, and shift to EventEmitter.

[aqilc]

also this class was so broken it was completely unusable

[aqilc]

*the Stream class

[117]

I've been wanting to redo the Stream class from the ground up. Skimmed a bit really like these changes! EventEmitter makes way more sense. Going to go over the files in detail and merge later tonight or tomorrow when i've got time.

[aqilc]

Ok. Please remove all setTimeouts btw, they are complete crap to use when doing request based things. PLEASE use the request API itself to check for timeouts and other stuff but setTimeout is a complete no-no.

[aqilc]

Ok, please review my changes. I fixed everything for the most part and it should be finished :D

[aqilc]

I was also thinking to add a types property to the class so we can customize it a bit, and making the stream host restricted to BaseURL + a simpler way to add host.

[aqilc]

Oh, i removed the message queueing... should i add it back?

[117]

Taking a look now.

[117]

I like your style, seeing a few ways of doing things I haven't seen elsewhere.

[aqilc]

I really like clean, understandable code and hate repetition, even though it takes up more space >.< Thanks!!!

[aqilc]

Is there anything to fix?

[aqilc]

i know i shouldn't discuss personal matters here but how was your trip?

[117]

~i know i shouldn't discuss personal matters here but how was your trip?~

😆 crossed an entire state and the scenery is much better than where I was before. so i'd say well worth the drive

[117]

Also running into an issue with the Stream, it seems to be authenticating before the connection is opened:

// Sends an authentication request
this.connection.send({ // line 64
    action: 'authenticate',
    data: { key_id: client.options.key, secret_key: client.options.secret },
});

throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');

Perhaps move the authentication request inside the 'open' event?

[aqilc]

OH RIGHT

[aqilc]

I'm actually so dumb xddd

[117]

I'm actually so dumb xddd

Nah it's a simple fix lol. I've done way worse.

[aqilc]

So should I add anything else? I could add the message queuing back even though you really should just use the event

[117]

So should I add anything else? I could add the message queuing back ~even though you really should just use the event~

Message queueing was for if the client dc'd and was in the middle of reconnecting, the message wouldn't get lost. But that's on the client user now to deal with if they so wish.

Checking a few more things then i'll let you know.

[aqilc]

Also, your trip sounded nice. I'm guessing you are finally taking your vacation after the pandemic thing toned down(still serious but at least the public is over it). xd

[aqilc]

Oh ok. That's what I was referring to btw. it will probably reduce performance a tiny bit too xd

[117]

One more thing, JSON.stringify before sending messages? Websocket doesn't like the object form apparently.

this.connection.send(
    JSON.stringify({
        action: 'authenticate',
        data: { key_id: client.options.key, secret_key: client.options.secret },
    })
)

Without stringify-ing I get this:

TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be one of type string, Buffer, ArrayBuffer, Array, or Array-like Object. Received type object

[aqilc]

Oh yes please. I didn't do any tests yet, and it's showing clearly.

[aqilc]

Ok, done.

[117]

Lol all good, our testing needs a revamp next. It's not very useful at the moment.

[aqilc]

Yes xd

[aqilc]

Can you merge and write the tests? I can't do this stuff well.

[117]

It's technically a string now but is not valid JSON so no responses are being returned from the websocket server.

[117]

Are you opposed to using JSON.stringify()?

[aqilc]

OHHH right i forgot to add strings to property names. I keep being dumb. Also, I'm always opposed to writing unnecessary functions since I'm used to performance requirements made by my computer, and like to optimize where possible.

[aqilc]

I took apart JSON.stringify once, and it's a bunch of unnecessary processing for small objects, since it's made to handle giant pieces of data.

[aqilc]

I was actually frustrated about not taking out spaces before and you gave me an excuse to xd. I'm still not satisfied since i left another 3 but ¯_(ツ)_/¯

[117]

The object.toString() method on the returned object doesn't tell us the error so perhaps JSON.stringify() should be used there?

Error: There was an error in authorizing your websocket connection. Object received: [object Object]

The response object in this scenario looks like this when stringified:

{"stream":"authorization","data":{"action":"authenticate","message":"access key verification failed","status":"unauthorized"}}

[117]

It is written in stream.ts as object.ToString() which prints [object Object] to console, not the object fields. :L

[aqilc]

Yes, I fixed it.

[aqilc]

Ahhh, I really should have tested it myself.......

[117]

Works now.

[aqilc]

Ok. Also, please find another issue as I missed my chance before.

[117]

Let's just merge it and bump the version a full 0.1? Work on it with some paper trade testing throughout the week.

[aqilc]

Wait, just adding a few things.

[aqilc]

Ok, done.

[aqilc]

I think we should update the main client before the version update too... since it's also requires a major upgrade.

[aqilc]

Also, I think this is production ready after this + client, so an upgrade to 1.0.0 would be more fitting

[117]

I'm gonna merge what's here so far and bump the version by 0.1, then we do client rework next and go straight to a release at 1.0.0.

[aqilc]

Yup :D