[#IP-368] table service unhandled exception

[BurnedMarshal]

List of Changes

• check response.isSuccessful only when Error falsy

Motivation and Context

On some scenarios the Azure table service returns an Error and a null response to the callbacks of insertEntity and deleteEntity. When this happens our code throw an unhandled exception trying to access to response.isSuccessful.

Application insights logs show the following error:

Exception while executing function: Functions.GetMessage node exited with code 1
   'Ingestion endpoint could not be reached. This batch of telemetry items has been lost. Use Disk Retry Caching to enable resending of failed telemetry. Error:',,  Error: getaddrinfo ENOTFOUND dc.services.visualstudio.com,LanguageWorkerConsoleLog[error]
Worker 00e991ee-XXXX-XXXX-XXXX-e2683f821aff uncaught exception (learn more: https://go.microsoft.com/fwlink/?linkid=2097909 ):
TypeError: Cannot read property 'isSuccessful' of null     at D:\home\site\wwwroot\dist\utils\storage.js:20:124 
at finalCallback (D:\home\site\wwwroot\node_modules\azure-storage\lib\services\table\tableservice.js:1323:9)
at D:\home\site\wwwroot\node_modules\azure-storage\lib\common\services\storageserviceclient.js:1016:11 
at processResponseCallback (D:\home\site\wwwroot\node_modules\azure-storage\lib\services\table\tableservice.js:1336:5)     at Request.processResponseCallback [as _callback]
(D:\home\site\wwwroot\node_modules\azure-storage\lib\common\services\storageserviceclient.js:329:13)
at self.callback (D:\home\site\wwwroot\node_modules\request\request.js:185:22)
at Request.emit (events.js:315:20)     at Request.onRequestError (D:\home\site\wwwroot\node_modules\request\request.js:877:8)
at ClientRequest.clsBind (D:\home\site\wwwroot\node_modules\cls-hooked\context.js:172:17)
at ClientRequest.emit (events.js:327:22)

The same error could occurs on functions-admin, functions-cgn, functions-services.

How Has This Been Tested?

unit test

Screenshots (if appropriate):

Types of changes

• Chore (nothing changes by a user perspective)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

[pagopa-github-bot]

	Warnings
⚠️	Please include a Pivotal story at the beginning of the PR title (see below).

Example of PR titles that include pivotal stories:

• single story: [#123456] my PR title
• multiple stories: [#123456,#123457,#123458] my PR title

Generated by 🚫 dangerJS

[balanza]

Should we assume such methods return ServiceResponse | null?

[BurnedMarshal]

Should we assume such methods return ServiceResponse | null?

Yes, but a specific typings is uneffective without strict=true or strictNullCheck=true options in tsconfig.

[balanza]

I don't see this solution to be solid, either. Consider the usage in here:

const { e1: resultOrError, e2: sResponse } = await insertEntityHandler(/* ... */)

as a matter of fact, resultOrError can be null-ish.

[BurnedMarshal]

I don't see this solution to be solid, either. Consider the usage in here:

const { e1: resultOrError, e2: sResponse } = await insertEntityHandler(/* ... */)

as a matter of fact, resultOrError can be null-ish.

This is a very good point. I try to handle better possible null values for error and response in 897ca70
If you think that this change is enough I can add some tests.

[balanza]

LGTM

I've added a couple of test scenarios in here, just to be sure.

I think the solution is solid enough for our needs.