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

Fix #258: Fixing crash when changing network while opening TTL #267

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

araujoarthur0
Copy link
Collaborator

Related issue

Closes #258: Fixing crash when changing network while opening TTL

Context / Background

While changing network and opening TTL at the same time, a network err exception is thrown and never caught anywhere.

What change is being introduced by this PR?

This PR adds a global catcher just for this exception.

How will this be tested?

Forced the same conditions that caused the issue and didn't see it happen :)

@codecov
Copy link

codecov bot commented Jun 19, 2020

Codecov Report

Merging #267 into master will increase coverage by 1.56%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #267      +/-   ##
==========================================
+ Coverage   55.08%   56.64%   +1.56%     
==========================================
  Files          13       13              
  Lines        1220     1234      +14     
  Branches      218      221       +3     
==========================================
+ Hits          672      699      +27     
+ Misses        480      468      -12     
+ Partials       68       67       -1     
Impacted Files Coverage Δ
main.js 0.00% <0.00%> (ø)
js/calendar.js 0.00% <0.00%> (ø)
src/preferences.js 0.00% <0.00%> (ø)
js/user-preferences.js 92.23% <0.00%> (+8.39%) ⬆️
js/window-aux.js 100.00% <0.00%> (+100.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e9db00c...9342e1e. Read the comment docs.

main.js Outdated Show resolved Hide resolved
@tupaschoal
Copy link
Collaborator

I mean, it doesn't really fiiiix, it just hides the exception and exits gracefully, I wonder if we can or should pursue fixing it, as it's a corner case.

@araujoarthur0
Copy link
Collaborator Author

Yeah, since the stack trace doesn't really say anything, I couldn't be sure if there is one thing in the code that is leading to this error.
My guess is the node/electron process is the cause here.
Since the tool doesn't really depend on network, I don't see a good reason to pursue it.

Copy link
Collaborator

@tupaschoal tupaschoal left a comment

Choose a reason for hiding this comment

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

Yeah, I guess we don't really need to pursue it. Can you add a changelog entry, please?

@araujoarthur0
Copy link
Collaborator Author

Added

@tupaschoal tupaschoal merged commit 252cbc7 into thamara:master Jun 19, 2020
@tupaschoal tupaschoal added this to the v1.5.3 milestone Jun 23, 2020
@araujoarthur0 araujoarthur0 deleted the network-err branch November 8, 2020 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when changing network
2 participants