Return 500 when DB is not connected

[fgalan]

(Comes from issue #559)

If database is not connected and any API will be requested then it will return "500 internal server error" and in logs it will display "database is not connected".

CC: @SwatiNEC

[fgalan]

According to #559 (comment), PR #560 addresses this issue.

@SwatiNEC could you merge master into PR #560 branch (solving any possible merging conflict during the process) so we can see how it looks like, please?

[Gauravp-NEC]

@fgalan , I have tried to re-base master branch with the PR #560 in my environment but it did not solve the issue. There is no handling of error after the reconnectTries value is exhaust. I think as a approach to fix this issue we should first reduce the reconnectTries to 10 instead of 1e3 so that user should not wait for so much time to find that MongoDB is not connected and Internal Server Error 500 should occur after the 10 reconnectTries. Please let me know your opinion :)

[AlvaroVega]

Fixed (reconnectTries and reconnectInterval default values according with default values in mongodb driver) in #572

[fgalan]

Fixed in #572

Just to clarify, what has been fixed in PR #572 is to reduce reconnectionTries to a smaller value (30) with an inter-reconnection time of 1 second (instead of 5 seconds).

Thus I understand you can not resync PR #560 with master and address this issue. Tell us if you need something else, please.

[Gauravp-NEC]

@AlvaroVega @fgalan , I have checked the PR #572. There is no error handling message in the PR when MongoDB is not connected and any API will be requested.

[fgalan]

@AlvaroVega @fgalan , I have checked the PR #572. There is no error handling message in the PR when MongoDB is not connected and any API will be requested.

As far as I understand, the error handling functionality to return 500 when DB is not connection is part of @SwatiNEC's PR #560 (see #559 (comment))

Is my understanding not correct? Could you clarify what PR #560 is really doing?

[Gauravp-NEC]

As far as I understand, the error handling functionality to return 500 when DB is not connection is part of @SwatiNEC's PR #560 (see #559 (comment))

Is my understanding not correct? Could you clarify what PR #560 is really doing?

@fgalan,Your understanding is correct. The error handling functionality to return 500 when DB is not connection is part of PR #560, but now the PR does not sync with the master branch.

[Gauravp-NEC]

@fgalan , I am looking forward to implement the error handling functionality to return 500 when DB is not connected.

[fgalan]

...but now the PR does not sync with the master branch.

I understand you mean that there are merging conflicts when trying to merge master into PR branch. This is usual business with git and the solution is to solve these conflicts. You can even to do using github, using the "Resolve conflict" button in the PR:

[fgalan]

@fgalan , I am looking forward to implement the error handling functionality to return 500 when DB is not connected.

Great! Thanks!

I understand you would coordinate with @SwatiNEC's (the author of PR #560)

[Gauravp-NEC]

@fgalan , I have raised the PR: #574 to fix this issue.

[fgalan]

After merging PR #583 this issue can be definitively closed.

Big thanks to @Gauravp-NEC @SwatiNEC @mapedraza for the team work implement this!

[chandradeep11]

@fgalan @mapedraza
Thanks for your work on this issue.
Now we are verifying issue #559 and #570 with updated code of PR #583 in our environment.

Can you please let us know the plan of next release of STH-Comet which includes this fix.

[fgalan]

Can you please let us know the plan of next release of STH-Comet which includes this fix.

If this night our CI e2e regression goes ok, a new version (2.9.0) will be released tomorrow.

[Gauravp-NEC]

@fgalan @mapedraza , We have tested the updated code of PR #583 in our environment. Few test scenarios are working fine but one test scenario is not working as per our expected.

1. Make sure that STH server and MongoDB are running and connected together.
2. Stop the MongoDB Container.
3. Start the MongoDB container when the reconnectTries value is exhausted.
4. Execute the GET API.

Our expected test result :

1. MongoDB should connect to STH server.
2. GET API should respond with attribute values.

Actual test result :

1. MongoDB is not connected to STH server.
2. GET API respond with:

statusCode: 500
error: Internal Sever Error
message: MongoDB is not connected

The expected result was successfully working in the PR #574 due to the
client.connect() function in else condition.

if (isConnectionAlive()) {
       fetchCollection(databaseName, isAggregated, shouldTruncate, shouldCreate, collectionName, callback);
   } else {
       client.connect(function(err) {
           if(err) {
               return callback(err,null);
           }
       });
   }
}

[fgalan]

@fgalan @mapedraza , We have tested the updated code of PR #583 in our environment. Few test scenarios are working fine but one test scenario is not working as per our expected.

1. Make sure that STH server and MongoDB are running and connected together.

2. Stop the MongoDB Container.

3. Start the MongoDB container when the` reconnectTries` value is exhausted.

4. Execute the GET API.

Our expected test result :

1. MongoDB should connect to STH server.

2. GET API should respond with attribute values.

Actual test result :

1. MongoDB is not connected to STH server.

2. GET API respond with:

statusCode: 500
error: Internal Sever Error
message: MongoDB is not connected

The expected result was successfully working in the PR #574 due to the client.connect() function in else condition.

if (isConnectionAlive()) {
       fetchCollection(databaseName, isAggregated, shouldTruncate, shouldCreate, collectionName, callback);
   } else {
       client.connect(function(err) {
           if(err) {
               return callback(err,null);
           }
       });
   }
}

Please open a new issue with this topic, so we can discuss in detail (the current issue has been "exhausted" in 2.9.0 and it is now closed).