Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix_for_OrionCB Payload loss #1408

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix_for_OrionCB Payload loss #1408

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6c9a125
Fix_for_OrionCB
KeshavSoni2511 Jul 21, 2023
8c011da
Fix_done
KeshavSoni2511 Aug 11, 2023
2b8c6f2
Rebased
KeshavSoni2511 Aug 11, 2023
7c64418
RemoveLintError
KeshavSoni2511 Aug 14, 2023
30f1af4
Merge remote-tracking branch 'upstream/master' into Fix_for_OrionCB
KeshavSoni2511 Aug 22, 2023
d1086d2
RebasedAndErrorFix
KeshavSoni2511 Aug 22, 2023
f9fd537
Rebased
KeshavSoni2511 Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGES_NEXT_RELEASE
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
- Fix: Payload measures are lost after reconnection with Orion (#1407)
- Fix: check array access in extractVariables of jexlPlugin when bidirectionalPlugin is enabled
- Fix: explicitAttrs of device was tainted even if not defined
- Fix: do not include static, lazy and commands from group to device to avoid duplicate them in device (#1377)
Expand All @@ -7,4 +8,4 @@
- Fix: use 'options=upsert' when update ngsiv2 CB entities and appendMode is enabled (#956)
- Fix: do not propagate group config (timestamp and explicitAttrs) to autoprovisioned devices (at database level) (#1377)
- Fix: appendMode at general level (config.js / env var) changes its default from false to true
- Fix: remove sensitive MongoDB connection parameters from log traces (remove 'option' object from logs)
- Fix: remove sensitive MongoDB connection parameters from log traces (remove 'option' object from logs)
26 changes: 22 additions & 4 deletions lib/request-shim.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
searchParams: options.searchParams || options.qs,
headers: options.headers,
throwHttpErrors: options.throwHttpErrors || false,
retry: options.retry || 0,
retry: options.retry || 5,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taken into account this retry parameter, I wonder if the loop based in retry_count is really needed. Could you include got library documentation about this retry option, please?

Copy link
Contributor Author

@KeshavSoni2511 KeshavSoni2511 Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fgalan , I include this retry count to make the default value less than httpOptions.retry so that the agent can retry 5 times before error.

Same here in https://github.com/telefonicaid/iotagent-json/blob/ef782f8f89ea2d0769dc36ecf9d96c63ef82cc17/lib/bindings/AMQPBinding.js#L130
and numRetried is used.

And please reply if I have to code it in camel case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://github.com/sindresorhus/got#documentation

By default, Got will retry on failure. To disable this option, set options.retry.limit to 0.

Thus, I'd say that the only needed modification is to unhardwire the 0 value currently used, so it can be configured (by config.js and env var).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, modifications from L98 to L130 wouldn't be necessary, as far as I understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fgalan , I have changed the default value but the agent was not retrying to connect to orion CB when it was down. Thats why I have to code to make it retry 5 times and also through an error as it was initially doing but after retrying 5 times after a particular interval of time. Thanks for the reply.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that got library is not working properly? As far as I understand, the got library should deal with this.

https://github.com/sindresorhus/got/blob/main/documentation/7-retry.md shows a pretty sophisticated configuration. I'd suggest to see how it works and use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fgalan , I just want to say to make that work we need some value to make it work 5 times so that it comes out of loop after max retries as it is implemented also in

https://github.com/telefonicaid/iotagent-json/blob/ef782f8f89ea2d0769dc36ecf9d96c63ef82cc17/lib/bindings/AMQPBinding.js#L130

and line 145

responseType: options.responseType || 'json'
};

Expand Down Expand Up @@ -95,18 +95,36 @@
*
*/

let retry_count = 0; //default retry_count value

Check failure on line 98 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Identifier 'retry_count' is not in camel case
function request(options, callback) {
const httpOptions = getOptions(options);
logger.debug(context, 'Options: %s', JSON.stringify(options, null, 4));
got(options.url || options.uri, httpOptions)
.then((response) => {
logger.debug(context, 'Response %s', JSON.stringify(response.body, null, 4));
retry_count = 0;

Check failure on line 105 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Identifier 'retry_count' is not in camel case
return callback(null, response, response.body);
})
.catch((error) => {

Check failure on line 108 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Expected to return a value at the end of arrow function
logger.debug(context, 'Error: %s', JSON.stringify(util.inspect(error), null, 4));
return callback(error);
if (retry_count == 0) {

Check failure on line 109 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Identifier 'retry_count' is not in camel case

Check failure on line 109 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Expected '===' and instead saw '=='
logger.debug(context, 'Error: %s', JSON.stringify(util.inspect(error), null, 4));
}
if (error.code == 'ECONNREFUSED') {

Check failure on line 112 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Expected '===' and instead saw '=='
if (retry_count < httpOptions.retry) {

Check failure on line 113 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Identifier 'retry_count' is not in camel case
retry_count++;

Check failure on line 114 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Identifier 'retry_count' is not in camel case
console.log('Retrying connection', JSON.stringify(retry_count));
return setTimeout(request, 2000, options, callback);
}
//retrun the error
if (retry_count == httpOptions.retry) {

Check failure on line 119 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Identifier 'retry_count' is not in camel case

Check failure on line 119 in lib/request-shim.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript 

Expected '===' and instead saw '=='
retry_count = 0;
return callback(error);
}
}
else{
return callback(error);
}
});
}

module.exports = request;
module.exports = request;
Loading