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 "Internet connection lost" errors #1913

Merged
merged 6 commits into from
Apr 21, 2021
Merged

Fix "Internet connection lost" errors #1913

merged 6 commits into from
Apr 21, 2021

Conversation

UniversalSuperBox
Copy link
Member

@UniversalSuperBox UniversalSuperBox commented Apr 14, 2021

Fixes #1847, see there for more discussion of the issue.

(Done) Currently a draft PR. I haven't tested this fully on a Windows or macOS machine.

(Done) Requires ubports/promise-android-tools#56
(Done) Requires https://gitlab.com/ubports/installer/android-tools-bin is released

Windows 10 includes working adb drivers for many devices. The drivers we
recommend people install are often more broken than these included ones.

Fixes #1708
Prior to this change, it was possible for errors initializing or waiting
on plugins to go undetected. Errors in wait() would bubble up to the
"no-network" error, which has been even more confusing for users.

Related, but does not fix:
#1847
If Heimdall is started on Windows without the Microsoft Visual C++ 2012
Redistributable package installed, it will exit with status 3221225781.

Since we're treating errors in wait() as fatal as of 3fa043c (and
reinforced that decision in 4f3899c), catch and notify the user of this
situation. They can choose to ignore it if they aren't installing on a
Samsung device.

I had to make some changes to the promise chains in the plugin index
and in core.js. The changes to the plugin index caused several unit
tests which were relying on very exact return values to fail, so they
had to be refactored.

Some tests didn't accurately describe or test their stated goals, so I
fixed them up.

This improves logging quite a lot.
Using 'npm install' causes npm to install the newest possible
dependencies based on package.json. 'npm ci' uses package-lock.json
instead. This ensures that releases are reproducible and so are test
failures.
@codecov
Copy link

codecov bot commented Apr 16, 2021

Codecov Report

Merging #1913 (63dab24) into master (31f330b) will increase coverage by 0.33%.
The diff coverage is 89.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1913      +/-   ##
==========================================
+ Coverage   71.95%   72.28%   +0.33%     
==========================================
  Files          25       25              
  Lines         845      884      +39     
==========================================
+ Hits          608      639      +31     
- Misses        237      245       +8     
Impacted Files Coverage Δ
src/lib/mainEvent.js 32.58% <50.00%> (-0.75%) ⬇️
src/core/core.js 66.66% <72.22%> (-3.34%) ⬇️
src/core/plugins/index.js 98.18% <96.77%> (-1.82%) ⬇️
src/core/plugins/adb/plugin.js 78.87% <100.00%> (ø)
src/core/plugins/heimdall/plugin.js 61.76% <100.00%> (+18.28%) ⬆️
src/core/plugins/plugin.js 100.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 31f330b...63dab24. Read the comment docs.

@UniversalSuperBox
Copy link
Member Author

I've had successful installations on the Nexus 5, OnePlus One, and Volla Phone on macOS, Windows, and Linux (but not the Volla Phone on macOS because fastboot is broken there).

@UniversalSuperBox UniversalSuperBox marked this pull request as ready for review April 17, 2021 02:35
@UniversalSuperBox
Copy link
Member Author

I've uploaded the artifacts from the GitHub Actions run to https://nc.ubports.com/index.php/s/g7sB3xdRfPRsA53 for further testing.

@mariogrip mariogrip merged commit 5d31f45 into master Apr 21, 2021
@mariogrip mariogrip deleted the fix-windows-macos branch April 21, 2021 12:57
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.

"Internet connection lost" appears immediately when starting installer on macOS or Windows
2 participants