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

Further hardened retry behavior for direct connect auth errors #102

Merged
merged 1 commit into from
Dec 27, 2023
Merged

Further hardened retry behavior for direct connect auth errors #102

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions src/kumo-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@ export class KumoApi {
} else {
this.log.warn('Kumo API: response error from device: %d %s',
response.status, response.statusText);
if(attempt_number < 2 && (await this.checkSecurityToken(true))) {
return this.directRequest(post_data, serial, attempt_number + 1);
}
Comment on lines +474 to +476
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra guard to re-auth for a generic error too. Just to harden this as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fjs21 Just pinging to see if you have any feedback for improvement for me on this one! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fjs21 Did you get a chance to look at this PR?

return null;
}
} catch(error) {
Expand All @@ -483,8 +486,8 @@ export class KumoApi {
return null;
}

if (!data || data == '{ _api_error: \'device_authentication_error\' }') {
this.log.warn('Kumo API: error direct querying device: %s.', serial);
if(!data || '_api_error' in data) {
this.log.warn('Kumo API: error direct querying device: %s; %s.', serial, data);
Comment on lines +489 to +490
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's one more hole I noticed in operation - it appears that api_error responses aren't getting caught by this line and are falling through to the return and being caught by the caller function with an error:

Kumo API: bad response from queryDevice_Direct - { _api_error: 'device_authentication_error' }

Once again, really tough to test locally. @fjs21 does this look like the correct way to check if the json response contains an _api_error?

if(attempt_number < 2 && (await this.checkSecurityToken(true))) {
return this.directRequest(post_data, serial, attempt_number + 1);
}
Expand Down
Loading