Skip to content

Commit 5d15757

Browse files
habibalkhabbazhabibalkhabbaz
authored
fix: fixed the error handler & improve stability (#448)
1 parent 70b8d0c commit 5d15757

20 files changed

+878
-637
lines changed

‎CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
All notable changes to this project will be documented in this file.
44

5+
## [Unreleased]
6+
7+
- Improved error handler and stability. Thanks [@habibalkhabbaz](https://github.com/habibalkhabbaz) - [#448](https://github.com/chrisleekr/binance-trading-bot/pull/448)
8+
59
## [0.0.88] - 2022-07-24
610

711
- Added TRADINGVIEW_LOG_LEVEL. Thanks [@azorpax](https://github.com/azorpax) - [#436](https://github.com/chrisleekr/binance-trading-bot/pull/436)

‎app/__tests__/error-handler.test.js

+151-93
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable global-require */
2+
// eslint-disable-next-line max-classes-per-file
23
describe('error-handler', () => {
34
let config;
45

@@ -29,80 +30,136 @@ describe('error-handler', () => {
2930
process.on = jest.fn().mockReturnValue(true);
3031
});
3132

32-
describe('triggers process.on', () => {
33-
beforeEach(() => {
34-
const { runErrorHandler } = require('../error-handler');
35-
runErrorHandler(mockLogger);
36-
});
33+
describe('errorHandlerWrapper', () => {
34+
[
35+
{
36+
label: 'Error -1001',
37+
code: -1001,
38+
sendSlack: false,
39+
featureToggleNotifyDebug: false
40+
},
41+
{
42+
label: 'Error -1021',
43+
code: -1021,
44+
sendSlack: false,
45+
featureToggleNotifyDebug: true
46+
},
47+
{
48+
label: 'Error ECONNRESET',
49+
code: 'ECONNRESET',
50+
sendSlack: false,
51+
featureToggleNotifyDebug: false
52+
},
53+
{
54+
label: 'Error ECONNREFUSED',
55+
code: 'ECONNREFUSED',
56+
sendSlack: false,
57+
featureToggleNotifyDebug: true
58+
},
59+
{
60+
label: 'Error something else - with notify debug',
61+
code: 'something',
62+
sendSlack: true,
63+
featureToggleNotifyDebug: true
64+
},
65+
{
66+
label: 'Error something else - without notify debug',
67+
code: 'something',
68+
sendSlack: true,
69+
featureToggleNotifyDebug: false
70+
}
71+
].forEach(errorInfo => {
72+
describe(`${errorInfo.label}`, () => {
73+
beforeEach(async () => {
74+
config.get = jest.fn(key => {
75+
if (key === 'featureToggle.notifyDebug') {
76+
return errorInfo.featureToggleNotifyDebug;
77+
}
78+
return null;
79+
});
3780

38-
it('with unhandledRejection', () => {
39-
expect(process.on).toHaveBeenCalledWith(
40-
'unhandledRejection',
41-
expect.any(Function)
42-
);
81+
const { errorHandlerWrapper } = require('../error-handler');
82+
await errorHandlerWrapper(mockLogger, 'WhateverJob', () => {
83+
throw new (class CustomError extends Error {
84+
constructor() {
85+
super();
86+
this.code = errorInfo.code;
87+
this.message = `${errorInfo.code}`;
88+
}
89+
})();
90+
});
91+
});
92+
93+
if (errorInfo.sendSlack) {
94+
it('triggers slack.sendMessage', () => {
95+
expect(mockSlack.sendMessage).toHaveBeenCalled();
96+
});
97+
} else {
98+
it('does not trigger slack.sendMessage', () => {
99+
expect(mockSlack.sendMessage).not.toHaveBeenCalled();
100+
});
101+
}
102+
});
43103
});
44104

45-
it('with uncaughtException', () => {
46-
expect(process.on).toHaveBeenCalledWith(
47-
'uncaughtException',
48-
expect.any(Function)
49-
);
105+
describe(`redlock error`, () => {
106+
beforeEach(async () => {
107+
config.get = jest.fn(_key => null);
108+
109+
const { errorHandlerWrapper } = require('../error-handler');
110+
await errorHandlerWrapper(mockLogger, 'WhateverJob', () => {
111+
throw new (class CustomError extends Error {
112+
constructor() {
113+
super();
114+
this.code = 500;
115+
this.message = `redlock:lock-XRPBUSD`;
116+
}
117+
})();
118+
});
119+
});
120+
121+
it('do not trigger slack.sendMessagage', () => {
122+
expect(mockSlack.sendMessage).not.toHaveBeenCalled();
123+
});
50124
});
51125
});
52126

53-
[
54-
{
55-
label: 'Error -1001',
56-
code: -1001,
57-
sendSlack: false,
58-
featureToggleNotifyDebug: false
59-
},
60-
{
61-
label: 'Error -1021',
62-
code: -1021,
63-
sendSlack: false,
64-
featureToggleNotifyDebug: true
65-
},
66-
{
67-
label: 'Error ECONNRESET',
68-
code: 'ECONNRESET',
69-
sendSlack: false,
70-
featureToggleNotifyDebug: false
71-
},
72-
{
73-
label: 'Error ECONNREFUSED',
74-
code: 'ECONNREFUSED',
75-
sendSlack: false,
76-
featureToggleNotifyDebug: true
77-
},
78-
{
79-
label: 'Error something else - with notify debug',
80-
code: 'something',
81-
sendSlack: true,
82-
featureToggleNotifyDebug: true
83-
},
84-
{
85-
label: 'Error something else - without notify debug',
86-
code: 'something',
87-
sendSlack: true,
88-
featureToggleNotifyDebug: false
89-
}
90-
].forEach(errorInfo => {
91-
describe(`${errorInfo.label}`, () => {
127+
describe('runErrorHandler', () => {
128+
describe('triggers process.on', () => {
129+
beforeEach(() => {
130+
const { runErrorHandler } = require('../error-handler');
131+
runErrorHandler(mockLogger);
132+
});
133+
134+
it('with unhandledRejection', () => {
135+
expect(process.on).toHaveBeenCalledWith(
136+
'unhandledRejection',
137+
expect.any(Function)
138+
);
139+
});
140+
141+
it('with uncaughtException', () => {
142+
expect(process.on).toHaveBeenCalledWith(
143+
'uncaughtException',
144+
expect.any(Function)
145+
);
146+
});
147+
});
148+
149+
describe(`when uncaughtException received without notifyDebug`, () => {
92150
beforeEach(async () => {
93151
config.get = jest.fn(key => {
94152
if (key === 'featureToggle.notifyDebug') {
95-
return errorInfo.featureToggleNotifyDebug;
153+
return false;
96154
}
97155
return null;
98156
});
99157

100158
process.on = jest.fn().mockImplementation((event, error) => {
101159
if (event === 'uncaughtException') {
102160
error({
103-
message: errorInfo.label,
104-
code: errorInfo.code,
105-
stack: errorInfo.code
161+
message: `something-unexpected`,
162+
code: 1000
106163
});
107164
}
108165
});
@@ -111,53 +168,54 @@ describe('error-handler', () => {
111168
runErrorHandler(mockLogger);
112169
});
113170

114-
if (errorInfo.sendSlack) {
115-
it('triggers slack.sendMessage', () => {
116-
expect(mockSlack.sendMessage).toHaveBeenCalled();
117-
});
118-
} else {
119-
it('does not trigger slack.sendMessage', () => {
120-
expect(mockSlack.sendMessage).not.toHaveBeenCalled();
121-
});
122-
}
123-
});
124-
});
125-
126-
describe(`redlock error`, () => {
127-
beforeEach(async () => {
128-
process.on = jest.fn().mockImplementation((event, error) => {
129-
if (event === 'uncaughtException') {
130-
error({
131-
message: `redlock:bot-lock:XRPBUSD`,
132-
code: 500
133-
});
134-
}
171+
it('triggers slack.sendMessage', () => {
172+
expect(mockSlack.sendMessage).toHaveBeenCalled();
135173
});
136-
137-
const { runErrorHandler } = require('../error-handler');
138-
runErrorHandler(mockLogger);
139174
});
140175

141-
it('do not trigger slack.sendMessagage', () => {
142-
expect(mockSlack.sendMessage).not.toHaveBeenCalled();
143-
});
144-
});
176+
describe(`when uncaughtException received with notifyDebug`, () => {
177+
beforeEach(async () => {
178+
config.get = jest.fn(key => {
179+
if (key === 'featureToggle.notifyDebug') {
180+
return true;
181+
}
182+
return null;
183+
});
145184

146-
describe(`any warning received on unhandledRejection`, () => {
147-
it('do not trigger slack.sendMessage', async () => {
148-
expect(() => {
149185
process.on = jest.fn().mockImplementation((event, error) => {
150-
if (event === 'unhandledRejection') {
186+
if (event === 'uncaughtException') {
151187
error({
152-
message: `redlock:bot-lock:XRPBUSD`,
153-
code: 500
188+
message: `something-unexpected`,
189+
code: 1000
154190
});
155191
}
156192
});
157193

158194
const { runErrorHandler } = require('../error-handler');
159195
runErrorHandler(mockLogger);
160-
}).toThrow(`redlock:bot-lock:XRPBUSD`);
196+
});
197+
198+
it('triggers slack.sendMessage', () => {
199+
expect(mockSlack.sendMessage).toHaveBeenCalled();
200+
});
201+
});
202+
203+
describe(`when unhandledRejection received`, () => {
204+
it('throws an error', async () => {
205+
expect(() => {
206+
process.on = jest.fn().mockImplementation((event, error) => {
207+
if (event === 'unhandledRejection') {
208+
error({
209+
message: `something-unhandled`,
210+
code: 2000
211+
});
212+
}
213+
});
214+
215+
const { runErrorHandler } = require('../error-handler');
216+
runErrorHandler(mockLogger);
217+
}).toThrow(`something-unhandled`);
218+
});
161219
});
162220
});
163221
});

‎app/__tests__/server.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('server', () => {
3838
require('../server');
3939
});
4040

41-
it('triggers errorHandler.run', () => {
41+
it('triggers runErrorHandler', () => {
4242
expect(mockRunErrorHandler).toHaveBeenCalled();
4343
});
4444

‎app/binance/__tests__/orders.test.js

+12-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
/* eslint-disable global-require */
2-
32
describe('orders.js', () => {
43
let binanceMock;
54
let loggerMock;
@@ -9,6 +8,7 @@ describe('orders.js', () => {
98

109
let mockUpdateGridTradeLastOrder;
1110
let mockGetOpenOrdersFromAPI;
11+
let mockErrorHandlerWrapper;
1212

1313
beforeEach(async () => {
1414
jest.clearAllMocks().resetModules();
@@ -18,6 +18,16 @@ describe('orders.js', () => {
1818
loggerMock = logger;
1919
cacheMock = cache;
2020
mongoMock = mongo;
21+
22+
mockErrorHandlerWrapper = jest
23+
.fn()
24+
.mockImplementation((_logger, _job, callback) => {
25+
callback();
26+
});
27+
28+
jest.mock('../../error-handler', () => ({
29+
errorHandlerWrapper: mockErrorHandlerWrapper
30+
}));
2131
});
2232

2333
describe('syncOpenOrders', () => {
@@ -106,9 +116,7 @@ describe('orders.js', () => {
106116

107117
loggerMock.error = jest.fn().mockResolvedValue(true);
108118

109-
mockGetOpenOrdersFromAPI = jest.fn().mockRejectedValue({
110-
error: 'error thrown'
111-
});
119+
mockGetOpenOrdersFromAPI = jest.fn().mockResolvedValue(true);
112120

113121
const spyOnSetInterval = jest.spyOn(global, 'setInterval');
114122
spyOnClearInterval = jest.spyOn(global, 'clearInterval');

‎app/binance/__tests__/tickers.test.js

+11
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,20 @@ describe('tickers.js', () => {
1010
let mockExecuteTrailingTrade;
1111

1212
let mockWebsocketTickersClean;
13+
let mockErrorHandlerWrapper;
1314

1415
beforeEach(() => {
1516
jest.clearAllMocks().resetModules();
17+
18+
mockErrorHandlerWrapper = jest
19+
.fn()
20+
.mockImplementation((_logger, _job, callback) =>
21+
Promise.resolve(callback())
22+
);
23+
24+
jest.mock('../../error-handler', () => ({
25+
errorHandlerWrapper: mockErrorHandlerWrapper
26+
}));
1627
});
1728

1829
describe('setupTickersWebsocket', () => {

‎app/binance/candles.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ const setupCandlesWebsocket = async (logger, symbols) => {
5858
};
5959

6060
/**
61-
* Retrieve ATH candles for symbols from Binance API
61+
* Retrieve candles for symbols from Binance API
6262
*
6363
* @param {*} logger
6464
* @param {string[]} symbols

0 commit comments

Comments
 (0)