Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

Add additional websocket message types #606

Merged
merged 7 commits into from Jan 3, 2022

Conversation

hjoelr
Copy link
Contributor

@hjoelr hjoelr commented Sep 17, 2021

The Coinbase API supports some additional websocket types. I'm adding them so they can be used natively in TypeScript.

@bennycode
Copy link
Owner

Hey @hjoelr, I am happy to add additional WebSocket types but we also need to have tests for the events when they occur.

@hjoelr
Copy link
Contributor Author

hjoelr commented Sep 20, 2021

@bennycode Okay. I've started to look at how to do that. I'm thinking I would add more it(...) calls under inside the describe('subscribe'...) section?

describe('subscribe', () => {
function mockWebSocketResponse(
done: DoneFn,
channels: WebSocketChannel | WebSocketChannel[],
payload: Object
): WebSocketClient {
server.on('connection', ws => {
ws.on('message', (message: string) => {
const request = JSON.parse(message) as WebSocketRequest;
if (request.type === WebSocketRequestType.SUBSCRIBE) {
// Send subscription confirmation
server.clients.forEach(client =>
client.send(
JSON.stringify({
channels: request.channels,
type: WebSocketResponseType.SUBSCRIPTIONS,
})
)
);
// Send event for subscription
server.clients.forEach(client => client.send(JSON.stringify(payload)));
}
if (request.type === WebSocketRequestType.UNSUBSCRIBE) {
// Send unsubscribe confirmation
server.clients.forEach(client => client.send(JSON.stringify(emptySubscriptions)));
}
});
});
const ws = createWebSocketClient();
ws.on(WebSocketEvent.ON_SUBSCRIPTION_UPDATE, subscriptions => {
// Disconnect when there are no more open subscriptions
if (subscriptions.channels.length === 0) {
ws.disconnect();
}
});
ws.on(WebSocketEvent.ON_CLOSE, done);
ws.on(WebSocketEvent.ON_MESSAGE_ERROR, (wsError: WebSocketErrorMessage) => done.fail(wsError.message));
// Send subscription once the WebSocket is ready
ws.on(WebSocketEvent.ON_OPEN, () => ws.subscribe(channels));
return ws;
}
it('receives typed messages from "status" channel', (done: DoneFn) => {
const channel = {
name: WebSocketChannelName.STATUS,
};
const ws = mockWebSocketResponse(done, channel, statusPayload);
ws.on(WebSocketEvent.ON_MESSAGE_STATUS, message => {
expect(message.currencies[2].details.sort_order).toBe(48);
expect(message.products[72].id).toBe('XRP-USD');
ws.unsubscribe(channel);
});
ws.connect();
});
it('receives typed messages from "ticker" channel', done => {
const channel = {
name: WebSocketChannelName.TICKER,
product_ids: ['BTC-USD'],
};
const ws = mockWebSocketResponse(done, channel, tickerBTCUSD);
ws.on(WebSocketEvent.ON_MESSAGE_TICKER, tickerMessage => {
expect(tickerMessage.trade_id).toBe(3526965);
ws.unsubscribe(channel);
});
ws.connect();
});
it('receives typed messages from multiple "matches" channels', done => {
const channels = [
{
name: WebSocketChannelName.MATCHES,
product_ids: ['BTC-USD'],
},
];
const ws = mockWebSocketResponse(done, channels, matchesBTCUSD);
ws.on(WebSocketEvent.ON_MESSAGE_MATCHES, message => {
expect(message.trade_id).toBe(9713921);
ws.unsubscribe(channels);
});
ws.connect();
});
it('receives typed error messages', done => {
server.on('connection', ws => {
ws.on('message', (message: string) => {
const request = JSON.parse(message);
if (request.type === WebSocketRequestType.SUBSCRIBE) {
const response = JSON.stringify({
message: 'Failed to subscribe',
reason: 'user channel requires authentication',
type: WebSocketResponseType.ERROR,
});
server.clients.forEach(client => {
client.send(response);
});
}
});
});
const ws = createWebSocketClient();
ws.on(WebSocketEvent.ON_MESSAGE_ERROR, errorMessage => {
expect(errorMessage.type).toBe(WebSocketResponseType.ERROR);
done();
});
ws.on(WebSocketEvent.ON_OPEN, () => {
ws.subscribe({
name: WebSocketChannelName.USER,
product_ids: ['BTC-USD'],
});
});
ws.connect();
});
});

@bennycode
Copy link
Owner

Hi @hjoelr, sorry for my late reply but I am currently on vacation (back to work next week). To add tests it's best to add more it blocks (one per message type test). If you change the it to fit, then only this test will be executed when running yarn test, which makes it pretty convenient to implement a new test. But don't forget to turn fit back into it when committing your final changes.

A full test run will also generate a "coverage/index.html" from where you can see which lines of code are covered by your test.

Here is a good example how to check for a specific message type:

it('receives typed messages from "status" channel', (done: DoneFn) => {
const channel = {
name: WebSocketChannelName.STATUS,
};
const ws = mockWebSocketResponse(done, channel, statusPayload);
ws.on(WebSocketEvent.ON_MESSAGE_STATUS, message => {
expect(message.currencies[2].details.sort_order).toBe(48);
expect(message.products[72].id).toBe('XRP-USD');
ws.unsubscribe(channel);
});
ws.connect();
});

@bennycode bennycode marked this pull request as draft October 10, 2021 08:07
@bennycode
Copy link
Owner

Hi @hjoelr, I converted your PR to a draft because it's still in progress.

@hjoelr
Copy link
Contributor Author

hjoelr commented Oct 10, 2021

Okay, no problem. I just haven't been able to get time to finish this off. 😔 Hopefully soon.

@bennycode
Copy link
Owner

Sure, take your time! There is no urgency with this one. 🍵

@hjoelr hjoelr force-pushed the feature/additional-ws-messages branch from 5595d80 to 997cd5b Compare December 19, 2021 03:28
@bennycode
Copy link
Owner

Hey @hjoelr, can I help you with adding the missing tests? 🙂

@hjoelr
Copy link
Contributor Author

hjoelr commented Dec 28, 2021

@bennycode Um, good question 😊. I recently brought my branch back into sync with your version and was planning on figuring out how to do this. I just need to take the time to learn how to mock in Jasmine. I hope to eventually submit some additional PRs and need to learn how to do this. I wouldn't mind help, but I also don't want you to have to spend time to write tests I should be writing 🙄.

@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2022

Codecov Report

Merging #606 (972d1ec) into main (6c79874) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #606   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           19        19           
  Lines          509       530   +21     
  Branches        39        39           
=========================================
+ Hits           509       530   +21     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/client/WebSocketClient.ts 100.00% <100.00%> (ø)

@hjoelr hjoelr force-pushed the feature/additional-ws-messages branch from 3b35fcc to af26f6a Compare January 3, 2022 06:04
@hjoelr
Copy link
Contributor Author

hjoelr commented Jan 3, 2022

@bennycode I think this PR is ready for review and merge. Thanks for bearing with me on this one!

@hjoelr hjoelr marked this pull request as ready for review January 3, 2022 06:07
@bennycode bennycode merged commit 6bf86e4 into bennycode:main Jan 3, 2022
@hjoelr hjoelr deleted the feature/additional-ws-messages branch January 3, 2022 21:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants