Skip to content

Commit ca850cd

Browse files
dimitrovmaksimdimitrovmaksim
authored
Return appropriate http codes (#793)
* Return appropriate http code Signed-off-by: Maksim Dimitrov <[email protected]> * Fix tests Signed-off-by: Maksim Dimitrov <[email protected]> * Don't use single letter variable names Signed-off-by: Maksim Dimitrov <[email protected]> * Update api tests Signed-off-by: Maksim Dimitrov <[email protected]> Signed-off-by: Maksim Dimitrov <[email protected]>
1 parent 2a45e98 commit ca850cd

File tree

5 files changed

+140
-104
lines changed

5 files changed

+140
-104
lines changed

‎packages/server/src/koaJsonRpc/index.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
} from './lib/RpcError';
3737
import Koa from 'koa';
3838
import { Registry } from 'prom-client';
39+
import { JsonRpcError } from '@hashgraph/json-rpc-relay';
3940

4041
const hasOwnProperty = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop);
4142
dotenv.config({ path: path.resolve(__dirname, '../../../../../.env') });
@@ -99,18 +100,21 @@ export default class KoaJsonRpc {
99100
ctx.request.method !== 'POST'
100101
) {
101102
ctx.body = jsonResp(body.id || null, new InvalidRequest(), undefined);
103+
ctx.status = 400;
102104
return;
103105
}
104106

105107
if (!this.registry[body.method]) {
106108
ctx.body = jsonResp(body.id, new MethodNotFound(), undefined);
109+
ctx.status = 400;
107110
return;
108111
}
109112

110113
const methodName = body.method;
111114
const methodTotalLimit = this.registryTotal[methodName];
112115
if (this.ratelimit.shouldRateLimit(ctx.ip, methodName, methodTotalLimit)) {
113116
ctx.body = jsonResp(body.id, new IPRateLimitExceeded(methodName), undefined);
117+
ctx.status = 409;
114118
return;
115119
}
116120

@@ -119,13 +123,18 @@ export default class KoaJsonRpc {
119123
} catch (e: any) {
120124
if (e instanceof InvalidParamsError) {
121125
ctx.body = jsonResp(body.id, new InvalidParamsError(e.message), undefined);
126+
ctx.status = 400;
122127
return;
123128
}
124129
ctx.body = jsonResp(body.id, new InternalError(e.message), undefined);
130+
ctx.status = 500;
125131
return;
126132
}
127133

128134
ctx.body = jsonResp(body.id, null, result);
135+
if (result instanceof JsonRpcError) {
136+
ctx.status = (result.code == -32603) ? 500 : 400;
137+
}
129138
};
130139
}
131140

‎packages/server/src/koaJsonRpc/lib/RpcResponse.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919
*/
2020

2121
export default function jsonResp(id, error, result) {
22-
const o: any = {};
22+
const response: any = {};
23+
2324
if (error && result) {
2425
throw new Error('Mutually exclusive error and result exist');
2526
}
@@ -29,7 +30,7 @@ export default function jsonResp(id, error, result) {
2930
}
3031

3132
if (typeof result !== 'undefined') {
32-
o.result = result;
33+
response.result = result;
3334
} else if (error) {
3435
if (typeof error.code !== 'number') {
3536
throw new TypeError(`Invalid error code type ${typeof error.code}`);
@@ -39,12 +40,12 @@ export default function jsonResp(id, error, result) {
3940
throw new TypeError(`Invalid error message type ${typeof error.message}`);
4041
}
4142

42-
o.error = error;
43+
response.error = error;
4344
} else {
4445
throw new Error('Missing result or error');
4546
}
4647

47-
o.jsonrpc = '2.0';
48-
o.id = id;
49-
return o;
48+
response.jsonrpc = '2.0';
49+
response.id = id;
50+
return response;
5051
}

‎packages/server/tests/clients/relayClient.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,12 @@ export default class RelayClient {
7474
async callUnsupported(methodName: string, params: any[], requestId?: string) {
7575
const requestIdPrefix = Utils.formatRequestIdMessage(requestId);
7676
try {
77-
const res = await this.call(methodName, params, requestId);
78-
this.logger.trace(`${requestIdPrefix} [POST] to relay '${methodName}' with params [${params}] returned ${JSON.stringify(res)}`);
79-
Assertions.expectedError();
77+
await this.call(methodName, params, requestId);
8078
} catch (err) {
81-
Assertions.unsupportedResponse(err);
82-
return err;
79+
this.logger.trace(`${requestIdPrefix} [POST] to relay '${methodName}' with params [${params}] returned ${err.body}`);
80+
const response = JSON.parse(err.body);
81+
Assertions.unsupportedResponse(response);
82+
return response;
8383
}
8484
};
8585

@@ -122,7 +122,7 @@ export default class RelayClient {
122122

123123
/**
124124
* @param requestId
125-
*
125+
*
126126
* Returns the result of eth_gasPrice as a Number.
127127
*/
128128
async gasPrice(requestId?: string): Promise<number> {

‎packages/server/tests/helpers/prerequisite.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ const RELAY_URL = process.env.E2E_RELAY_HOST || LOCAL_RELAY_URL;
1414
process.env['NETWORK_NODE_IMAGE_TAG'] = '0.32.0';
1515
process.env['HAVEGED_IMAGE_TAG'] = '0.32.0';
1616
process.env['MIRROR_IMAGE_TAG'] = '0.70.1';
17-
17+
1818
console.log(`Docker container versions, services: ${process.env['NETWORK_NODE_IMAGE_TAG']}, mirror: ${process.env['MIRROR_IMAGE_TAG']}`);
19-
19+
2020
console.log('Installing local node...');
2121
shell.exec(`npm install @hashgraph/hedera-local -g`);
22-
22+
2323
console.log('Starting local node...');
2424
shell.exec(`hedera start -d`);
2525
console.log('Hedera Hashgraph local node env started');
@@ -30,4 +30,4 @@ const RELAY_URL = process.env.E2E_RELAY_HOST || LOCAL_RELAY_URL;
3030
console.log(`Start relay on port ${process.env.SERVER_PORT}`);
3131
app.listen({ port: process.env.SERVER_PORT });
3232
}
33-
})();
33+
})();

‎packages/server/tests/integration/server.spec.ts‎

Lines changed: 114 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -197,124 +197,146 @@ describe('RPC Server', async function() {
197197
});
198198

199199
it('should execute "web3_sha"', async function() {
200-
const res = await this.testClient.post('/', {
201-
'id': '2',
202-
'jsonrpc': '2.0',
203-
'method': 'web3_sha',
204-
'params': [null]
205-
});
206-
207-
BaseTest.methodNotFoundCheck(res);
200+
try{
201+
await this.testClient.post('/', {
202+
'id': '2',
203+
'jsonrpc': '2.0',
204+
'method': 'web3_sha',
205+
'params': [null]
206+
});
207+
} catch (error) {
208+
BaseTest.methodNotFoundCheck(error.response);
209+
}
208210
});
209211

210212
it('should execute "net_peerCount"', async function() {
211-
const res = await this.testClient.post('/', {
212-
'id': '2',
213-
'jsonrpc': '2.0',
214-
'method': 'net_peerCount',
215-
'params': [null]
216-
});
217-
218-
BaseTest.methodNotFoundCheck(res);
213+
try {
214+
await this.testClient.post('/', {
215+
'id': '2',
216+
'jsonrpc': '2.0',
217+
'method': 'net_peerCount',
218+
'params': [null]
219+
});
220+
} catch (error) {
221+
BaseTest.methodNotFoundCheck(error.response);
222+
}
219223
});
220224

221225
it('should execute "eth_submitHashrate"', async function() {
222-
const res = await this.testClient.post('/', {
223-
'id': '2',
224-
'jsonrpc': '2.0',
225-
'method': 'eth_submitHashrate',
226-
'params': [null]
227-
});
228-
229-
BaseTest.unsupportedJsonRpcMethodChecks(res);
226+
try {
227+
await this.testClient.post('/', {
228+
'id': '2',
229+
'jsonrpc': '2.0',
230+
'method': 'eth_submitHashrate',
231+
'params': [null]
232+
});
233+
} catch (error) {
234+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
235+
}
230236
});
231237

232238
it('should execute "eth_signTypedData"', async function() {
233-
const res = await this.testClient.post('/', {
234-
'id': '2',
235-
'jsonrpc': '2.0',
236-
'method': 'eth_signTypedData',
237-
'params': [null]
238-
});
239-
240-
BaseTest.methodNotFoundCheck(res);
239+
try {
240+
await this.testClient.post('/', {
241+
'id': '2',
242+
'jsonrpc': '2.0',
243+
'method': 'eth_signTypedData',
244+
'params': [null]
245+
});
246+
} catch (error) {
247+
BaseTest.methodNotFoundCheck(error.response);
248+
}
241249
});
242250

243251
it('should execute "eth_signTransaction"', async function() {
244-
const res = await this.testClient.post('/', {
245-
'id': '2',
246-
'jsonrpc': '2.0',
247-
'method': 'eth_signTransaction',
248-
'params': [null]
249-
});
250-
251-
BaseTest.unsupportedJsonRpcMethodChecks(res);
252+
try {
253+
await this.testClient.post('/', {
254+
'id': '2',
255+
'jsonrpc': '2.0',
256+
'method': 'eth_signTransaction',
257+
'params': [null]
258+
});
259+
} catch (error) {
260+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
261+
}
252262
});
253263

254264
it('should execute "eth_sign"', async function() {
255-
const res = await this.testClient.post('/', {
256-
'id': '2',
257-
'jsonrpc': '2.0',
258-
'method': 'eth_sign',
259-
'params': [null]
260-
});
261-
262-
BaseTest.unsupportedJsonRpcMethodChecks(res);
265+
try {
266+
await this.testClient.post('/', {
267+
'id': '2',
268+
'jsonrpc': '2.0',
269+
'method': 'eth_sign',
270+
'params': [null]
271+
});
272+
} catch (error) {
273+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
274+
}
263275
});
264276

265277
it('should execute "eth_sendTransaction"', async function() {
266-
const res = await this.testClient.post('/', {
267-
'id': '2',
268-
'jsonrpc': '2.0',
269-
'method': 'eth_sendTransaction',
270-
'params': [null]
271-
});
272-
273-
BaseTest.unsupportedJsonRpcMethodChecks(res);
278+
try {
279+
await this.testClient.post('/', {
280+
'id': '2',
281+
'jsonrpc': '2.0',
282+
'method': 'eth_sendTransaction',
283+
'params': [null]
284+
});
285+
} catch (error) {
286+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
287+
}
274288
});
275289

276290
it('should execute "eth_protocolVersion"', async function() {
277-
const res = await this.testClient.post('/', {
278-
'id': '2',
279-
'jsonrpc': '2.0',
280-
'method': 'eth_protocolVersion',
281-
'params': [null]
282-
});
283-
284-
BaseTest.unsupportedJsonRpcMethodChecks(res);
291+
try {
292+
await this.testClient.post('/', {
293+
'id': '2',
294+
'jsonrpc': '2.0',
295+
'method': 'eth_protocolVersion',
296+
'params': [null]
297+
});
298+
} catch (error) {
299+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
300+
}
285301
});
286302

287303
it('should execute "eth_getProof"', async function() {
288-
const res = await this.testClient.post('/', {
289-
'id': '2',
290-
'jsonrpc': '2.0',
291-
'method': 'eth_getProof',
292-
'params': [null]
293-
});
294-
295-
BaseTest.methodNotFoundCheck(res);
304+
try {
305+
await this.testClient.post('/', {
306+
'id': '2',
307+
'jsonrpc': '2.0',
308+
'method': 'eth_getProof',
309+
'params': [null]
310+
});
311+
} catch (error) {
312+
BaseTest.methodNotFoundCheck(error.response);
313+
}
296314
});
297315

298316
it('should execute "eth_coinbase"', async function() {
299-
const res = await this.testClient.post('/', {
300-
'id': '2',
301-
'jsonrpc': '2.0',
302-
'method': 'eth_coinbase',
303-
'params': [null]
304-
});
305-
306-
BaseTest.unsupportedJsonRpcMethodChecks(res);
317+
try {
318+
await this.testClient.post('/', {
319+
'id': '2',
320+
'jsonrpc': '2.0',
321+
'method': 'eth_coinbase',
322+
'params': [null]
323+
});
324+
} catch (error) {
325+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
326+
}
307327
});
308328

309329
it('should execute "eth_getWork"', async function() {
310-
const res = await this.testClient.post('/', {
311-
'id': '2',
312-
'jsonrpc': '2.0',
313-
'method': 'eth_getWork',
314-
'params': [null]
315-
});
316-
317-
BaseTest.unsupportedJsonRpcMethodChecks(res);
330+
try {
331+
await this.testClient.post('/', {
332+
'id': '2',
333+
'jsonrpc': '2.0',
334+
'method': 'eth_getWork',
335+
'params': [null]
336+
});
337+
} catch (error) {
338+
BaseTest.unsupportedJsonRpcMethodChecks(error.response);
339+
}
318340
});
319341

320342
it('should execute "eth_maxPriorityFeePerGas"', async function() {
@@ -370,10 +392,14 @@ class BaseTest {
370392
}
371393

372394
static unsupportedJsonRpcMethodChecks(response) {
395+
expect(response.status).to.eq(400);
396+
expect(response.statusText).to.eq('Bad Request');
373397
this.errorResponseChecks(response, -32601, 'Unsupported JSON-RPC method');
374398
}
375399

376400
static methodNotFoundCheck(response) {
401+
expect(response.status).to.eq(400);
402+
expect(response.statusText).to.eq('Bad Request');
377403
this.errorResponseChecks(response, -32601, 'Method not found');
378404
}
379405
}

0 commit comments

Comments
 (0)