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.

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

Bugfix/crash on browsing for bad credentials #44

Merged
merged 6 commits into from
May 12, 2022
Merged

Conversation

ZmRZjFuDsDiR4qi5801F
Copy link
Contributor

This unhandled exception was hard to find.
The branch is missleading and has nothing todo with ipv6 and the crash is 100% reproducable.

  • Connect a DL Request with good credentials
  • Change the credentials to be bad
  • Open the DL Request config dialog and press the button -> crash of node

Copy link
Contributor

@krauskopf krauskopf left a comment

Choose a reason for hiding this comment

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

In general this looks like a good fix. Thanks for finding! Please have a look at the minor issue about logging directly to stdout.

ctrlx-config.js Outdated
.finally(() => ctrlx.logOut());
.finally(() => {
// We have to catch, because this fails for bad credentials
ctrlx.logOut().catch((err) => console.log('Failed to log out with error ' + err.message));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please check if RED.log.warn() would work here? I think, this would be the better option than logging directly to console. But I'm not sure if it works in this context. Also Message "Failed to log out when trying to browse with error" gives more information about what happened.

Copy link
Contributor

@krauskopf krauskopf left a comment

Choose a reason for hiding this comment

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

Nice fix. Thanks!

ctrlx-config.js Outdated
.finally(() => ctrlx.logOut());
.finally(() => {
// We have to catch, because this fails for bad credentials
ctrlx.logOut().catch((err) => console.log('Failed to log out with error ' + err.message));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please check if RED.log.warn() would work here? I think, this would be the better option than logging directly to console. But I'm not sure if it works in this context. Also Message "Failed to log out when trying to browse with error" gives more information about what happened.

@krauskopf krauskopf merged commit 05f05b3 into master May 12, 2022
@krauskopf krauskopf deleted the bugfix/ipv6-on-linux branch May 12, 2022 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants