Azure/azure-cosmos-js

CosmosClientOptions.connectionPolicy: Nested object retryOptions replaced instead of merged

jrauschenbusch opened this issue · 5 comments

Currently if one is providing a custom set of retryOptions the default will get lost due the following LOC:
https://github.com/Azure/azure-cosmos-js/blob/v3.0.3/src/CosmosClient.ts#L64

Expected: Nested merge logic instead of merely top level merge

Solution: Nested object assign like
https://www.npmjs.com/package/nested-object-assign

@jrauschenbusch Can you describe the behavior you see that led you to this? I am not able to repro this with the code below. It is definitely possible we are dropping retryOptions somewhere else in the code. Any other details you can provide will help me track it down.

const { CosmosClient } = require('@azure/cosmos')

const endpoint = 'https://your-account.documents.azure.com' // Add your endpoint
const key = '[database account masterkey]' // Add the masterkey of the endpoint
const client = new CosmosClient({
  endpoint,
  key,
  connectionPolicy: {
    retryOptions: {
      maxRetryAttemptCount: 1000
    }
  }
})

console.log(client.clientContext.connectionPolicy.retryOptions)
//=> { maxRetryAttemptCount: 1000 }
``

@southpolesteve I try to reify the problem with your example.

const { CosmosClient } = require('@azure/cosmos')

const endpoint = 'https://your-account.documents.azure.com
const key = '[database account masterkey]'
const client = new CosmosClient({
  endpoint,
  key,
  connectionPolicy: {
    retryOptions: {
      maxRetryAttemptCount: 1000
    }
  }
})

console.log(client.clientContext.connectionPolicy.retryOptions)
// => should lead to { maxRetryAttemptCount: 1000, fixedRetryIntervalInMilliseconds: 0, maxWaitTimeInSeconds: 30 } as defaultRetryOptions is part of defaultConnectionPolicy.

IMHO default options should persist as long as they are not explicitly overridden. But due to the non-nested merge using Object.assign(...) the remaining defaults fixedRetryIntervalInMilliseconds and maxWaitTimeInSeconds are not getting merged. As a result ResourceThrottleRetryPolicy #maxWaitTimeInMilliseconds is initialized with NaN which leads to a non-working retry policy.

https://github.com/Azure/azure-cosmos-js/blob/master/src/retry/resourceThrottleRetryPolicy.ts#L51

Ah! I see the issue now. Thanks. Will have a fix out soon.

Fixed in 3.0.4

thx for fixing it!