opensearch-project / opensearch-js

Node.js Client for OpenSearch
https://opensearch.org/docs/latest/clients/javascript/
Apache License 2.0
188 stars 122 forks source link

[BUG] undefined cluster_manager when running local mock #640

Open theahura opened 1 year ago

theahura commented 1 year ago

Caveat that I totally get if this is not the right place for this bug -- the 'opensearch mock' that I'm trying to build is not directly related to this repo, but I'm hoping someone here can help me out nonetheless.

What is the bug?

When running the opensearch client with a mocked connection, I get TypeError: Cannot read properties of undefined (reading 'cluster_manager').

Full trace:

@hearth/binaries.api:     TypeError: Cannot read properties of undefined (reading 'cluster_manager')                                 
@hearth/binaries.api:                                                                                                                                                                  
@hearth/binaries.api:       at defaultNodeFilter (../../../node_modules/@opensearch-project/opensearch/lib/Transport.js:679:17)
@hearth/binaries.api:       at ConnectionPool.getConnection (../../../node_modules/@opensearch-project/opensearch/lib/pool/ConnectionPool.js:214:13)
@hearth/binaries.api:       at Transport.getConnection (../../../node_modules/@opensearch-project/opensearch/lib/Transport.js:544:32)                  
@hearth/binaries.api:       at makeRequest (../../../node_modules/@opensearch-project/opensearch/lib/Transport.js:232:30)
@hearth/binaries.api:       at prepareRequest (../../../node_modules/@opensearch-project/opensearch/lib/Transport.js:530:9)
@hearth/binaries.api:       at Transport.request (../../../node_modules/@opensearch-project/opensearch/lib/Transport.js:534:5)
@hearth/binaries.api:       at IndicesApi.indicesExistsApi [as exists] (../../../node_modules/@opensearch-project/opensearch/api/api/indices.js:691:25)
@hearth/binaries.api:       at exists (../../libs/node/src/opensearch/provider.ts:66:18)                                                                                               
@hearth/binaries.api:       at opensearchProvider.connect (../../libs/node/src/runtime.ts:115:5)                             
@hearth/binaries.api:       at Object.factory [as getOpenSearchClient] (../../libs/common/src/singleton.ts:68:19)                    
@hearth/binaries.api:       at getOpenSearchClient (../../libs/node/src/mocktime.ts:53:36)
@hearth/binaries.api:       at call (../../../node_modules/@babel/runtime/helpers/regeneratorRuntime.js:44:17)
@hearth/binaries.api:       at Generator.tryCatch (../../../node_modules/@babel/runtime/helpers/regeneratorRuntime.js:125:22)
@hearth/binaries.api:       at Generator._invoke [as next] (../../../node_modules/@babel/runtime/helpers/regeneratorRuntime.js:69:21)
@hearth/binaries.api:       at next (../../../../node_modules/tslib/tslib.es6.js:118:58)

How can one reproduce the bug?

There's a pattern of mocking opensearch which attempts to use the elasticsearch mock client and trick the opensearch api to accept the mock. See: https://github.com/opensearch-project/opensearch-js/issues/192, or https://stackoverflow.com/questions/71358008/mock-opensearch-client-in-jest-unit-test

However, clearly there has been drift -- in particular, the following fails:

 16 import Mock from '@elastic/elasticsearch-mock';                                  
 18 import * as opensearch from '@opensearch-project/opensearch';                   
 23                                                                                 
 24 export type MockClient = Mock;
...
 41   const mock = new Mock();                                                       
 42                                                                                  
 43   // The following casts are necessary because the official Elasticsearch        
 44   // Connection mock has slightly different types than the OpenSearch client.    
 45   // Under the hood, the relevant code paths behave exactly identically, so      
 46   // we are mostly safe to ignore the type differences.                          
 47   // See: https://stackoverflow.com/questions/71358008/mock-opensearch-client-in-jest-unit-test
 48   // for an example of how this mock works.                                      
 49                                                                                  
 50   const client = new opensearch.Client({                                         
 51     node: awsConstants.OPENSEARCH_ENDPOINT,                                      
 52     Connection:                                                                 
 53       mock.getConnection() as unknown as typeof opensearch.Connection,           
 54   });                                                                            
 55                                                                                  
 56   mock.add(                                                                      
 57     {                                                                            
 58       method: 'HEAD',                                                            
 59       path: '*',                                                                 
 60     },                                                                           
 61     () => {                                                                      
 62       return '';                                                                 
 63     },                                                                           
 64   );                                                                             
 65                                                                                  
 66   client.indices.exists({ index: 'test' }).then(console.log);                    
 6                 

I dug into the code myself and, as best I can tell, the error is occurring because OpenSearch is looking for a node and there's no filtering information or roles present on the fake client, so the node search fails. Unfortunately just setting a fake nodeFilter also fails -- if the nodeFilter defaults to true, the code hangs; if false, it fails with a NoLivingConnectionsError

It's all running locally and faked, so I'd love if there was a way to disable the cluster management entirely. Is this possible from the client input parameters alone?

What is the expected behavior?

No error.

What is your host/environment?

Linux 20.04, OpenSearch 2.5

ScottGuymer commented 10 months ago

I'm also seeing this issue when trying to mock an OpenSearch instance using the elastic mocks library.

I also didn't find any workable solutions to it yet.

dblock commented 10 months ago

Do we need to fork https://www.npmjs.com/package/@elastic/elasticsearch-mock and add support for cluster manager? please feel free to do so! (that comes from https://opensearch.org/docs/latest/breaking-changes/#deprecate-non-inclusive-terms).