From bb9ba906991784618d29168c986a93849e4a4a98 Mon Sep 17 00:00:00 2001 From: George Raduta Date: Wed, 8 Jan 2025 10:57:36 +0100 Subject: [PATCH 1/4] [improv] Update grpcErrorToNativeError.js Use details instead of message field of the gRPC error struct --- .../Backend/errors/grpcErrorToNativeError.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Framework/Backend/errors/grpcErrorToNativeError.js b/Framework/Backend/errors/grpcErrorToNativeError.js index 8495b2396..06e0a6634 100644 --- a/Framework/Backend/errors/grpcErrorToNativeError.js +++ b/Framework/Backend/errors/grpcErrorToNativeError.js @@ -19,10 +19,11 @@ const { UnauthorizedAccessError } = require('./UnauthorizedAccessError.js'); /** * @typedef GrpcError - * also known as gRPC Status Object + * also known as gRPC Status Object https://grpc.github.io/grpc/node/grpc.html#~StatusObject * * @property {number} code - code of the gRPC Status object - * @property {string} message - message of the gRPC Status object + * @property {string} message - message of the gRPC Status object / includes code as well in the string + * @property {string} details - details of the gRPC Status object */ /** @@ -33,21 +34,21 @@ const { UnauthorizedAccessError } = require('./UnauthorizedAccessError.js'); * @returns {Error} */ const grpcErrorToNativeError = (error) => { - const { code, message } = error; + const { code, details } = error; switch (code) { case 3: - return new InvalidInputError(message); + return new InvalidInputError(details); case 4: - return new TimeoutError(message); + return new TimeoutError(details); case 5: - return new NotFoundError(message); + return new NotFoundError(details); case 7: - return new UnauthorizedAccessError(message); + return new UnauthorizedAccessError(details); case 14: - return new ServiceUnavailableError(message); + return new ServiceUnavailableError(details); default: - return new Error(message); + return new Error(details); } }; From 0085fa7b0b9a1ef2fb53842c58a3ad68fab3e631 Mon Sep 17 00:00:00 2001 From: George Raduta Date: Wed, 8 Jan 2025 11:08:07 +0100 Subject: [PATCH 2/4] Allow developer to opt for message instead of details of gRPC error --- .../Backend/errors/grpcErrorToNativeError.js | 17 ++--- .../errors/mocha-grpcErrorToNativeError.js | 63 ++++++++++++++++--- 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/Framework/Backend/errors/grpcErrorToNativeError.js b/Framework/Backend/errors/grpcErrorToNativeError.js index 06e0a6634..df4b4d17c 100644 --- a/Framework/Backend/errors/grpcErrorToNativeError.js +++ b/Framework/Backend/errors/grpcErrorToNativeError.js @@ -31,24 +31,25 @@ const { UnauthorizedAccessError } = require('./UnauthorizedAccessError.js'); * Code List source: https://grpc.github.io/grpc/core/md_doc_statuscodes.html * * @param {GrpcError} error - error object from gRPC Client library + * @param {Boolean} includeStatusCode - whether to error message field or details field * @returns {Error} */ -const grpcErrorToNativeError = (error) => { - const { code, details } = error; +const grpcErrorToNativeError = (error, includeStatusCode = false) => { + const { code, details, message } = error; switch (code) { case 3: - return new InvalidInputError(details); + return new InvalidInputError(includeStatusCode ? message : details); case 4: - return new TimeoutError(details); + return new TimeoutError(includeStatusCode ? message : details); case 5: - return new NotFoundError(details); + return new NotFoundError(includeStatusCode ? message : details); case 7: - return new UnauthorizedAccessError(details); + return new UnauthorizedAccessError(includeStatusCode ? message : details); case 14: - return new ServiceUnavailableError(details); + return new ServiceUnavailableError(includeStatusCode ? message : details); default: - return new Error(details); + return new Error(includeStatusCode ? message : details); } }; diff --git a/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js b/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js index 297a25489..bb01a66e8 100644 --- a/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js +++ b/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js @@ -23,15 +23,64 @@ const assert = require('assert'); describe('\'grpcErrorToNativeError\' test suite', () => { it('should successfully convert gRPC errors to native errors', () => { - assert.deepStrictEqual(grpcErrorToNativeError({ code: 3, message: 'invalid' }), new InvalidInputError('invalid')); - assert.deepStrictEqual(grpcErrorToNativeError({ code: 4, message: 'timeout' }), new TimeoutError('timeout')); - assert.deepStrictEqual(grpcErrorToNativeError({ code: 5, message: 'not-found' }), new NotFoundError('not-found')); - assert.deepStrictEqual(grpcErrorToNativeError({ code: 7, message: 'unauthorized' }), new UnauthorizedAccessError('unauthorized')); assert.deepStrictEqual( - grpcErrorToNativeError({ code: 14, message: 'service-unavailable' }), + grpcErrorToNativeError({ code: 3, message: '3: invalid', details: 'invalid' }), + new InvalidInputError('invalid'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 4, message: '4: timeout', details: 'timeout' }), + new TimeoutError('timeout'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 5, message: '5: not-found', details: 'not-found' }), + new NotFoundError('not-found'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 7, message: 'unauthorized', details: 'unauthorized' }), + new UnauthorizedAccessError('unauthorized'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 14, message: 'service-unavailable', details: 'service-unavailable' }), new ServiceUnavailableError('service-unavailable'), ); - assert.deepStrictEqual(grpcErrorToNativeError({ code: 100, message: 'standard-error' }), new Error('standard-error')); - assert.deepStrictEqual(grpcErrorToNativeError({ message: 'standard-error' }), new Error('standard-error')); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 100, message: 'standard-error', details: 'standard-error' }), + new Error('standard-error'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ message: 'standard-error', details: 'standard-error' }), + new Error('standard-error'), + ); + }); + + it('should successfully convert gRPC errors to native errors', () => { + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 3, message: '3: invalid', details: 'invalid' }, true), + new InvalidInputError('3: invalid'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 4, message: '4: timeout', details: 'timeout' }, true), + new TimeoutError('4: timeout'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 5, message: '5: not-found', details: 'not-found' }, true), + new NotFoundError('5: not-found'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 7, message: '7: unauthorized', details: 'unauthorized' }, true), + new UnauthorizedAccessError('7: unauthorized'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 14, message: '14: service-unavailable', details: 'service-unavailable' }, true), + new ServiceUnavailableError('14: service-unavailable'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ code: 100, message: '100: standard-error', details: 'standard-error' }, true), + new Error('100: standard-error'), + ); + assert.deepStrictEqual( + grpcErrorToNativeError({ message: 'code: standard-error', details: 'standard-error' }, true), + new Error('code: standard-error'), + ); }); }); From 6452a204a1003bbb90c4368c6acd4d73e98ac152 Mon Sep 17 00:00:00 2001 From: George Raduta Date: Wed, 8 Jan 2025 13:43:53 +0100 Subject: [PATCH 3/4] Add gRPC error codes and update tests --- .../Backend/errors/grpcErrorCodes.enum.js | 23 ++++ .../Backend/errors/grpcErrorToNativeError.js | 15 ++- .../errors/mocha-grpcErrorToNativeError.js | 126 +++++++++--------- 3 files changed, 97 insertions(+), 67 deletions(-) create mode 100644 Framework/Backend/errors/grpcErrorCodes.enum.js diff --git a/Framework/Backend/errors/grpcErrorCodes.enum.js b/Framework/Backend/errors/grpcErrorCodes.enum.js new file mode 100644 index 000000000..f1c05e2d0 --- /dev/null +++ b/Framework/Backend/errors/grpcErrorCodes.enum.js @@ -0,0 +1,23 @@ +/** + * @license + * Copyright CERN and copyright holders of ALICE O2. This software is + * distributed under the terms of the GNU General Public License v3 (GPL + * Version 3), copied verbatim in the file "COPYING". + * + * See http://alice-o2.web.cern.ch/license for full licensing information. + * + * In applying this license CERN does not waive the privileges and immunities + * granted to it by virtue of its status as an Intergovernmental Organization + * or submit itself to any jurisdiction. + */ + +const GrpcErrorCodes = Object.freeze({ + UNKNOWN: 2, + INVALID_INPUT: 3, + TIMEOUT: 4, + NOT_FOUND: 5, + UNAUTHORIZED_ACCESS: 7, + SERVICE_UNAVAILABLE: 14, +}); + +exports.GrpcErrorCodes = GrpcErrorCodes; diff --git a/Framework/Backend/errors/grpcErrorToNativeError.js b/Framework/Backend/errors/grpcErrorToNativeError.js index df4b4d17c..b39398e9a 100644 --- a/Framework/Backend/errors/grpcErrorToNativeError.js +++ b/Framework/Backend/errors/grpcErrorToNativeError.js @@ -16,14 +16,15 @@ const { NotFoundError } = require('./NotFoundError.js'); const { ServiceUnavailableError } = require('./ServiceUnavailableError.js'); const { TimeoutError } = require('./TimeoutError.js'); const { UnauthorizedAccessError } = require('./UnauthorizedAccessError.js'); +const { GrpcErrorCodes } = require('./grpcErrorCodes.enum.js'); /** * @typedef GrpcError * also known as gRPC Status Object https://grpc.github.io/grpc/node/grpc.html#~StatusObject * - * @property {number} code - code of the gRPC Status object - * @property {string} message - message of the gRPC Status object / includes code as well in the string + * @property {GrpcErrorCodes} code - code of the gRPC Status object * @property {string} details - details of the gRPC Status object + * @property {string} message - message of the gRPC Status object */ /** @@ -38,15 +39,15 @@ const grpcErrorToNativeError = (error, includeStatusCode = false) => { const { code, details, message } = error; switch (code) { - case 3: + case GrpcErrorCodes.INVALID_INPUT: return new InvalidInputError(includeStatusCode ? message : details); - case 4: + case GrpcErrorCodes.TIMEOUT: return new TimeoutError(includeStatusCode ? message : details); - case 5: + case GrpcErrorCodes.NOT_FOUND: return new NotFoundError(includeStatusCode ? message : details); - case 7: + case GrpcErrorCodes.UNAUTHORIZED_ACCESS: return new UnauthorizedAccessError(includeStatusCode ? message : details); - case 14: + case GrpcErrorCodes.SERVICE_UNAVAILABLE: return new ServiceUnavailableError(includeStatusCode ? message : details); default: return new Error(includeStatusCode ? message : details); diff --git a/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js b/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js index bb01a66e8..73acac49c 100644 --- a/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js +++ b/Framework/Backend/test/errors/mocha-grpcErrorToNativeError.js @@ -12,75 +12,81 @@ * or submit itself to any jurisdiction. */ +const assert = require('assert'); + const { grpcErrorToNativeError } = require('../../errors/grpcErrorToNativeError.js'); +const { GrpcErrorCodes: { + INVALID_INPUT, TIMEOUT, NOT_FOUND, UNAUTHORIZED_ACCESS, SERVICE_UNAVAILABLE, UNKNOWN, +} } = require('../../errors/grpcErrorCodes.enum.js'); const { InvalidInputError } = require('../../errors/InvalidInputError.js'); const { NotFoundError } = require('../../errors/NotFoundError.js'); const { ServiceUnavailableError } = require('../../errors/ServiceUnavailableError.js'); const { TimeoutError } = require('../../errors/TimeoutError.js'); const { UnauthorizedAccessError } = require('../../errors/UnauthorizedAccessError.js'); -const assert = require('assert'); - describe('\'grpcErrorToNativeError\' test suite', () => { - it('should successfully convert gRPC errors to native errors', () => { - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 3, message: '3: invalid', details: 'invalid' }), - new InvalidInputError('invalid'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 4, message: '4: timeout', details: 'timeout' }), - new TimeoutError('timeout'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 5, message: '5: not-found', details: 'not-found' }), - new NotFoundError('not-found'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 7, message: 'unauthorized', details: 'unauthorized' }), - new UnauthorizedAccessError('unauthorized'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 14, message: 'service-unavailable', details: 'service-unavailable' }), - new ServiceUnavailableError('service-unavailable'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 100, message: 'standard-error', details: 'standard-error' }), - new Error('standard-error'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ message: 'standard-error', details: 'standard-error' }), - new Error('standard-error'), - ); + const testCases = [ + { + grpcError: { code: UNKNOWN, message: `${UNKNOWN}: Error is unknown`, details: 'Error is unknown' }, + expectedError: Error, + expectedMessage: `${UNKNOWN}: Error is unknown`, + expectedDetails: 'Error is unknown', + }, + { + grpcError: { code: INVALID_INPUT, message: `${INVALID_INPUT}: Invalid input details`, details: 'Invalid input details' }, + expectedError: InvalidInputError, + expectedMessage: `${INVALID_INPUT}: Invalid input details`, + expectedDetails: 'Invalid input details', + }, + { + grpcError: { code: TIMEOUT, message: `${TIMEOUT}: Timeout occurred`, details: 'Timeout occurred' }, + expectedError: TimeoutError, + expectedMessage: `${TIMEOUT}: Timeout occurred`, + expectedDetails: 'Timeout occurred', + }, + { + grpcError: { code: NOT_FOUND, message: `${NOT_FOUND}: Not found`, details: 'Not found' }, + expectedError: NotFoundError, + expectedMessage: `${NOT_FOUND}: Not found`, + expectedDetails: 'Not found', + }, + { + grpcError: { code: UNAUTHORIZED_ACCESS, message: `${UNAUTHORIZED_ACCESS}: Unauthorized access`, details: 'Unauthorized access details' }, + expectedError: UnauthorizedAccessError, + expectedMessage: `${UNAUTHORIZED_ACCESS}: Unauthorized access`, + expectedDetails: 'Unauthorized access details', + }, + { + grpcError: { code: SERVICE_UNAVAILABLE, message: `${SERVICE_UNAVAILABLE}: Service unavailable`, details: 'Service unavailable details' }, + expectedError: ServiceUnavailableError, + expectedMessage: `${SERVICE_UNAVAILABLE}: Service unavailable`, + expectedDetails: 'Service unavailable details', + }, + { + grpcError: { code: 100, message: '100: Unknown Code and Error', details: 'Unknown Code and Error' }, + expectedError: Error, + expectedMessage: '100: Unknown Code and Error', + expectedDetails: 'Unknown Code and Error', + }, + ]; + + testCases.forEach(({ grpcError: { code, details }, expectedError, expectedDetails }) => { + it(`should convert gRPC error with code ${code} and pass DETAILS in error of type ${expectedError}`, () => { + assert.deepStrictEqual(grpcErrorToNativeError({ code, details }), new expectedError(expectedDetails)); + }); + }); + + testCases.forEach(({ grpcError: { code, message }, expectedError, expectedMessage }) => { + it(`should convert gRPC error with code ${code} and pass MESSAGE in error of type ${expectedError.name}`, () => { + assert.deepStrictEqual(grpcErrorToNativeError({ code, message }, true), new expectedError(expectedMessage)); + }); + }); + + it('should handle gRPC error with unknown code gracefully', () => { + assert.deepStrictEqual(grpcErrorToNativeError({ code: 999, details: 'unknown-error' }), new Error('unknown-error')); }); - it('should successfully convert gRPC errors to native errors', () => { - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 3, message: '3: invalid', details: 'invalid' }, true), - new InvalidInputError('3: invalid'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 4, message: '4: timeout', details: 'timeout' }, true), - new TimeoutError('4: timeout'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 5, message: '5: not-found', details: 'not-found' }, true), - new NotFoundError('5: not-found'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 7, message: '7: unauthorized', details: 'unauthorized' }, true), - new UnauthorizedAccessError('7: unauthorized'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 14, message: '14: service-unavailable', details: 'service-unavailable' }, true), - new ServiceUnavailableError('14: service-unavailable'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ code: 100, message: '100: standard-error', details: 'standard-error' }, true), - new Error('100: standard-error'), - ); - assert.deepStrictEqual( - grpcErrorToNativeError({ message: 'code: standard-error', details: 'standard-error' }, true), - new Error('code: standard-error'), - ); + it('should handle gRPC error without message gracefully', () => { + assert.deepStrictEqual(grpcErrorToNativeError({ code: 3 }), new InvalidInputError('')); }); }); From eba08c4213b643890ccb652aee5ea3af7421ca3b Mon Sep 17 00:00:00 2001 From: George Raduta Date: Wed, 8 Jan 2025 14:45:24 +0100 Subject: [PATCH 4/4] Export grpcCodes from framework --- Framework/Backend/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Framework/Backend/index.js b/Framework/Backend/index.js index c72631575..7585031f0 100644 --- a/Framework/Backend/index.js +++ b/Framework/Backend/index.js @@ -33,6 +33,7 @@ const { ServiceUnavailableError } = require('./errors/ServiceUnavailableError.js const { TimeoutError } = require('./errors/TimeoutError.js'); const { UnauthorizedAccessError } = require('./errors/UnauthorizedAccessError.js'); const { grpcErrorToNativeError } = require('./errors/grpcErrorToNativeError.js'); +const { GrpcErrorCodes } = require('./errors/grpcErrorCodes.enum.js'); const { updateAndSendExpressResponseFromNativeError, } = require('./errors/updateAndSendExpressResponseFromNativeError.js'); @@ -79,6 +80,8 @@ exports.TimeoutError = TimeoutError; exports.UnauthorizedAccessError = UnauthorizedAccessError; +exports.GrpcErrorCodes = GrpcErrorCodes; + exports.grpcErrorToNativeError = grpcErrorToNativeError; exports.updateAndSendExpressResponseFromNativeError = updateAndSendExpressResponseFromNativeError;