Skip to content

Commit ad94b8f

Browse files
Resolving comments
1 parent 1237ec1 commit ad94b8f

File tree

3 files changed

+55
-48
lines changed

3 files changed

+55
-48
lines changed

lib/core/CustomRequestHandler.js

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,17 @@
22

33
const logger = require('../util/loggerFactory');
44

5-
/*
6-
This class handles the custom requests made to existing playwright connection.
7-
This is kept as Singleton class because we want to maintain a map of commands for which
8-
we can resolve the response once we receive from Playwright server.
9-
Please don't change the singleton behaviour of this class.
10-
*/
5+
/**
6+
* Handles the custom requests made to existing playwright connection.
7+
* This class is implemented as a Singleton to maintain a map of commands for which
8+
* responses can be resolved once received from the Playwright server.
9+
* @class
10+
*/
1111
class CustomRequestHandler {
12+
/**
13+
* Creates an instance of CustomRequestHandler.
14+
* @constructor
15+
*/
1216
constructor() {
1317
if (!CustomRequestHandler.instance) {
1418
// Initialize the instance if it doesn't exist
@@ -19,7 +23,11 @@ class CustomRequestHandler {
1923
// Return the existing instance if it already exists
2024
return CustomRequestHandler.instance;
2125
}
22-
// Static method to get the single instance of the class
26+
27+
/**
28+
* Static method to get the single instance of the class.
29+
* @returns {CustomRequestHandler} The single instance of the CustomRequestHandler class.
30+
*/
2331
static getInstance() {
2432
if (!CustomRequestHandler.instance) {
2533
// Create a new instance if it doesn't exist
@@ -29,6 +37,10 @@ class CustomRequestHandler {
2937
return CustomRequestHandler.instance;
3038
}
3139

40+
/**
41+
* Checks if the custom request list is empty.
42+
* @returns {boolean} Returns true if the custom request list is empty, otherwise false.
43+
*/
3244
isCustomRequestListEmpty() {
3345
for (const prop in this.customRequestList) {
3446
if (this.customRequestList.hasOwnProperty(prop)) {
@@ -39,11 +51,10 @@ class CustomRequestHandler {
3951
return true;
4052
}
4153

42-
// only for specs purpose, don't use it in code.
43-
static resetInstance() {
44-
CustomRequestHandler.instance = null;
45-
}
46-
// Method to add an item to the list
54+
/**
55+
* Adds an item to the custom request list.
56+
* @param {string} request_id - The ID of the request to be added.
57+
*/
4758
addCustomRequest(request_id) {
4859
let resolveFunc;
4960
let rejectFunc;
@@ -58,9 +69,23 @@ class CustomRequestHandler {
5869
};
5970
logger.info(`Added request '${request_id}' to the customRequestList.`);
6071
}
61-
// Method to get the list items
72+
73+
/**
74+
* Gets the items in the custom request list.
75+
* @returns {Object} The custom request list.
76+
*/
6277
getList() {
6378
return this.customRequestList;
6479
}
80+
81+
/**
82+
* Resets the instance of the CustomRequestHandler class.
83+
* Only for testing purposes. Do not use it in production code.
84+
* @static
85+
*/
86+
static resetInstance() {
87+
CustomRequestHandler.instance = null;
88+
}
6589
}
90+
6691
module.exports = CustomRequestHandler;

lib/core/Proxy.js

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ const AlertManager = require('../util/AlertManager');
2323
const Instrumentation = require('../util/Instrumentation');
2424
const ErrorHandler = require('../util/ErrorHandler');
2525
const CustomRequestHandler = require('./CustomRequestHandler');
26+
const responseHeaders = {
27+
'content-type': 'application/json; charset=utf-8',
28+
accept: 'application/json',
29+
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
30+
}
2631

2732
/**
2833
* Proxy is the entrypoint and instantiates the context among the socket connection.
@@ -111,32 +116,20 @@ class Proxy {
111116
customReqInstance.customRequestList[commandId].promise
112117
.then((result) => {
113118
delete customReqInstance.customRequestList[commandId];
114-
response.writeHead(200, {
115-
'content-type': 'application/json; charset=utf-8',
116-
accept: 'application/json',
117-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
118-
});
119+
response.writeHead(200, responseHeaders);
119120
response.end(
120121
JSON.stringify({ status: 'success', value: result })
121122
);
122123
})
123124
.catch((err) => {
124125
delete customReqInstance.customRequestList[commandId];
125-
response.writeHead(500, {
126-
'content-type': 'application/json; charset=utf-8',
127-
accept: 'application/json',
128-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
129-
});
126+
response.writeHead(500, responseHeaders);
130127
response.end(JSON.stringify({ status: 'failure', value: err }));
131128
});
132129
});
133130
} catch (err) {
134131
logger.error(`Error while handling custom request ${err}`);
135-
response.writeHead(500, {
136-
'content-type': 'application/json; charset=utf-8',
137-
accept: 'application/json',
138-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
139-
});
132+
response.writeHead(500, responseHeaders);
140133
response.end(JSON.stringify({ status: 'failure', value: err.message }));
141134
}
142135
return;

test/core/proxy.test.js

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ const http = require('http');
88
const { assert } = require('console');
99
const proxyquire = require('proxyquire');
1010
const CustomRequestHandler = require('../../lib/core/CustomRequestHandler');
11+
const responseHeaders = {
12+
'content-type': 'application/json; charset=utf-8',
13+
accept: 'application/json',
14+
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
15+
};
1116

1217
describe('Proxy', () => {
1318
before(() => {
@@ -146,11 +151,7 @@ describe('Proxy', () => {
146151
);
147152
setTimeout(() => {
148153
assertChai.isTrue(
149-
responseMock.writeHead.calledOnceWith(200, {
150-
'content-type': 'application/json; charset=utf-8',
151-
accept: 'application/json',
152-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
153-
})
154+
responseMock.writeHead.calledOnceWith(200, responseHeaders)
154155
);
155156
assertChai.isTrue(
156157
responseMock.end.calledOnceWith(
@@ -218,11 +219,7 @@ describe('Proxy', () => {
218219
);
219220
setTimeout(() => {
220221
assertChai.isTrue(
221-
responseMock.writeHead.calledOnceWith(500, {
222-
'content-type': 'application/json; charset=utf-8',
223-
accept: 'application/json',
224-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
225-
})
222+
responseMock.writeHead.calledOnceWith(500, responseHeaders)
226223
);
227224
assertChai.isTrue(
228225
responseMock.end.calledOnceWith(
@@ -248,11 +245,7 @@ describe('Proxy', () => {
248245

249246
// Assertions for the outer catch block
250247
assertChai.isTrue(
251-
responseMock.writeHead.calledOnceWith(500, {
252-
'content-type': 'application/json; charset=utf-8',
253-
accept: 'application/json',
254-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
255-
})
248+
responseMock.writeHead.calledOnceWith(500, responseHeaders)
256249
);
257250
assertChai.isTrue(
258251
responseMock.end.calledOnceWith(
@@ -273,11 +266,7 @@ describe('Proxy', () => {
273266
this.proxy.requestHandler(request, this.response);
274267
expect(this.response.writeHead.calledOnce).to.be.equal(true);
275268
assert(
276-
this.response.writeHead.calledWith(200, {
277-
'content-type': 'application/json; charset=utf-8',
278-
accept: 'application/json',
279-
'WWW-Authenticate': 'Basic realm="WS Reconnect Proxy"',
280-
})
269+
this.response.writeHead.calledWith(200, responseHeaders)
281270
);
282271
expect(this.response.end.calledOnce).to.be.equal(true);
283272
assert(

0 commit comments

Comments
 (0)