Merge branch 'windows-uninstall-log'

4 files changed, 240 insertions, 146 deletions

diff --git a/dist-assets/windows/installer.nsh b/dist-assets/windows/installer.nsh
index 2407c0ffcf..935c390384 100644
--- a/dist-assets/windows/installer.nsh
+++ b/dist-assets/windows/installer.nsh
@@ -33,8 +33,9 @@
 !define DL_GENERAL_SUCCESS 0
 
 # Log targets
-!define LOG_FILE 0
-!define LOG_VOID 1
+!define LOG_INSTALL 0
+!define LOG_UNINSTALL 1
+!define LOG_VOID 2
 
 # Windows error codes
 !define ERROR_SERVICE_DOES_NOT_EXIST 1060
@@ -45,6 +46,7 @@
 !define MVSETUP_OK 0
 !define MVSETUP_ERROR 1
 !define MVSETUP_VERSION_NOT_OLDER 2
+!define MVSETUP_DAEMON_NOT_RUNNING 3
 
 # Override electron-builder generated application settings key.
 # electron-builder uses a GUID here rather than the application name.
@@ -247,12 +249,6 @@
 	Push $1
 	Push $2
 
-	Var /GLOBAL InstallService_Counter
-	Push 0
-	Pop $InstallService_Counter
-
-	InstallService_RegisterService:
-
 	log::Log "Running $\"mullvad-daemon$\" for it to self-register as a service"
 	nsExec::ExecToStack '"$INSTDIR\resources\mullvad-daemon.exe" --register-service'
 
@@ -263,50 +259,6 @@
 		StrCpy $R0 "Failed to install Mullvad service"
 		log::LogWithDetails $R0 $1
 
-		#
-		# Parse service error
-		#
-		string::Find $1 "(os error " 0
-		Pop $0
-
-		${If} $0 == -1
-			log::Log "Failed to parse service error"
-			Goto InstallService_return
-		${EndIf}
-
-		IntOp $0 $0 + 10
-
-		string::Find $1 ")" $0
-		Pop $2
-
-		IntOp $2 $2 - $0
-
-		${If} $2 < 1
-			log::Log "Failed to parse service error"
-			Goto InstallService_return
-		${EndIf}
-
-		StrCpy $0 $1 $2 $0
-
-		StrCpy $R0 "Service error code: $0"
-		log::Log $R0
-
-		#
-		# Forcibly kill old process if stuck
-		#
-		${If} $0 == ${ERROR_SERVICE_MARKED_FOR_DELETE}
-			log::Log "Attempt to forcibly kill stuck process"
-			nsExec::ExecToStack '"$SYSDIR\taskkill.exe" /f /fi "SERVICES eq mullvadvpn"'
-			Pop $0
-			Pop $1
-
-			# Retry service installation
-			IntOp $InstallService_Counter $InstallService_Counter + 1
-			${If} $InstallService_Counter < 2
-				Goto InstallService_RegisterService
-			${EndIf}
-		${EndIf}
-
 		Goto InstallService_return
 	${EndIf}
 
@@ -594,6 +546,8 @@
 #
 !macro ClearFirewallRules
 
+	log::Log "ClearFirewallRules()"
+
 	Push $0
 	Push $1
 
@@ -601,7 +555,11 @@
 	Pop $0
 	Pop $1
 
-	log::Log "Resetting firewall: $0 $1"
+	${If} $0 != ${MVSETUP_OK}
+		log::LogWithDetails "ClearFirewallRules() failed" $1
+	${Else}
+		log::Log "ClearFirewallRules() completed successfully"
+	${EndIf}
 
 	Pop $1
 	Pop $0
@@ -617,6 +575,8 @@
 #
 !macro ClearAccountHistory
 
+	log::Log "ClearAccountHistory()"
+
 	Push $0
 	Push $1
 
@@ -624,7 +584,11 @@
 	Pop $0
 	Pop $1
 
-	log::Log "Remove account history: $0 $1"
+	${If} $0 != ${MVSETUP_OK}
+		log::LogWithDetails "ClearAccountHistory() failed" $1
+	${Else}
+		log::Log "ClearAccountHistory() completed successfully"
+	${EndIf}
 
 	Pop $1
 	Pop $0
@@ -697,9 +661,10 @@
 	Var /GLOBAL BlockFilterResult
 	Var /GLOBAL PersistentBlockFilterResult
 
+	Push $1
 	Push $R0
 
-	log::Initialize ${LOG_FILE}
+	log::SetLogTarget ${LOG_INSTALL}
 
 	log::Log "Running installer for ${PRODUCT_NAME} ${VERSION}"
 	log::LogWindowsVersion
@@ -714,6 +679,19 @@
 	SetShellVarContext current
 	RMDir /r "$LOCALAPPDATA\mullvad-vpn-updater"
 
+	#
+	# Hack to check whether the uninstaller succeeded.
+	# This assumes that it got far enough to create a log file.
+	# Note that the uninstaller has already been replaced at this point.
+	#
+	SetShellVarContext all
+	IfFileExists "$LOCALAPPDATA\Mullvad VPN\uninstall.log" 0 customInstall_uninstaller_succeeded
+
+	MessageBox MB_OK "Failed to uninstall a previous version. Contact support or see the logs for more information."
+	Goto customInstall_abort_installation
+
+	customInstall_uninstaller_succeeded:
+
 	${MigrateCache}
 	${RemoveRelayCache}
 	${RemoveApiAddressCache}
@@ -761,18 +739,19 @@
 
 	${If} $BlockFilterResult == 0
 	${OrIf} $PersistentBlockFilterResult == 0
-		MessageBox MB_ICONEXCLAMATION|MB_YESNO "Do you wish to unblock your internet access? Doing so will leave you with an unsecure connection." IDNO customInstall_abortInstallation_skip_firewall_revert
+		MessageBox MB_ICONEXCLAMATION|MB_YESNO "Do you wish to unblock your internet access? Doing so will leave you with an unsecure connection." IDNO customInstall_abort_installation_skip_firewall_revert
 		${ExtractMullvadSetup}
 		${ClearFirewallRules}
 	${EndIf}
 
-	customInstall_abortInstallation_skip_firewall_revert:
+	customInstall_abort_installation_skip_firewall_revert:
 
 	Abort
 
 	customInstall_skip_abort:
 
 	Pop $R0
+	Pop $1
 
 !macroend
 
@@ -783,6 +762,115 @@
 ###############################################################################
 
 #
+# StopAndDeleteService
+#
+# Stops and deletes the service, attempting to forcibly kill it if necessary.
+#
+# Returns: 0 in $R0 on success. Otherwise, an error message in $R0 is returned.
+#
+!macro StopAndDeleteService
+
+	log::Log "StopAndDeleteService()"
+
+	Push $0
+	Push $1
+
+	nsExec::ExecToStack '"$SYSDIR\sc.exe" query mullvadvpn'
+
+	Pop $0
+	Pop $1
+
+	${If} $0 == ${ERROR_SERVICE_DOES_NOT_EXIST}
+		Goto StopAndDeleteService_success
+	${EndIf}
+
+	log::Log "Stopping Mullvad service"
+
+	nsExec::ExecToStack '"$SYSDIR\net.exe" stop mullvadvpn'
+
+	Pop $0
+	Pop $1
+
+	${If} $0 != 0
+		log::LogWithDetails "Failed to stop the service: $0" $1
+
+		# It may be possible to recover by force-killing the service
+		# This also "fails" with a generic error if the service isn't running
+	${EndIf}
+
+	# Copy over the daemon log from the old install for debugging purposes
+	SetShellVarContext all
+	CopyFiles /SILENT /FILESONLY "$LOCALAPPDATA\Mullvad VPN\daemon.log" "$LOCALAPPDATA\Mullvad VPN\old-install-daemon.log"
+
+	log::Log "Removing Mullvad service"
+	nsExec::ExecToStack '"$SYSDIR\sc.exe" delete mullvadvpn'
+
+	Pop $0
+	Pop $R0
+
+	${If} $0 != 0
+	${AndIf} $0 != ${ERROR_SERVICE_MARKED_FOR_DELETE}
+		log::Log "Failed to delete the service: $0"
+		Goto StopAndDeleteService_return_only
+	${EndIf}
+
+	Sleep 1000
+
+	#
+	# Forcibly kill the service (if marked for deletion)
+	#
+
+	Var /GLOBAL DeleteService_Counter
+	Push 0
+	Pop $DeleteService_Counter
+
+	StopAndDeleteService_check_delete:
+
+	nsExec::ExecToStack '"$SYSDIR\sc.exe" query mullvadvpn'
+
+	Pop $0
+	Pop $1
+
+	${If} $0 != ${ERROR_SERVICE_DOES_NOT_EXIST}
+		log::Log "Attempting to forcibly kill Mullvad service"
+
+		nsExec::ExecToStack '"$SYSDIR\taskkill.exe" /f /fi "SERVICES eq mullvadvpn"'
+		Pop $0
+		Pop $1
+
+		# Check again whether it was deleted
+		IntOp $DeleteService_Counter $DeleteService_Counter + 1
+		${If} $DeleteService_Counter < 3
+			Sleep 1000
+			Goto StopAndDeleteService_check_delete
+		${EndIf}
+
+		StrCpy $R0 "Failed to kill Mullvad service"
+		log::Log $R0
+		Goto StopAndDeleteService_return_only
+	${EndIf}
+
+	StopAndDeleteService_success:
+
+	Push 0
+	Pop $R0
+
+	StopAndDeleteService_return_only:
+
+	${If} $R0 == 0
+		log::Log "StopAndDeleteService() completed successfully"
+	${Else}
+		log::Log "StopAndDeleteService() failed"
+	${EndIf}
+
+	Pop $1
+	Pop $0
+
+!macroend
+
+!define StopAndDeleteService '!insertmacro "StopAndDeleteService"'
+
+#
 # customRemoveFiles
 #
 # This macro is activated just after the removal of files have started.
@@ -792,26 +880,27 @@
 
 	Push $0
 	Push $1
+	Push $R0
 
 	# Check command line arguments
 	Var /GLOBAL FullUninstall
 	Var /GLOBAL Silent
 	Var /GLOBAL NewVersion
 
+	log::SetLogTarget ${LOG_UNINSTALL}
+
+	log::Log "Running uninstaller for ${PRODUCT_NAME} ${VERSION}"
+
 	${GetParameters} $0
 	${GetOptions} $0 "/S" $1
 	${If} ${Errors}
 		Push 0
-		log::Initialize ${LOG_VOID}
 	${Else}
 		Push 1
-		log::Initialize ${LOG_FILE}
 	${EndIf}
 
 	Pop $Silent
 
-	log::Log "Running uninstaller for ${PRODUCT_NAME} ${VERSION}"
-
 	${ExtractMullvadSetup}
 
 	${If} $Silent == 1
@@ -839,62 +928,41 @@
 		nsExec::ExecToStack '"$TEMP\mullvad-setup.exe" prepare-restart'
 		Pop $0
 		Pop $1
-	${EndIf}
 
-	nsExec::ExecToStack '"$SYSDIR\net.exe" stop mullvadvpn'
-
-	# Discard return value
-	Pop $0
-	Pop $1
-
-	# Copy over the daemon log from the old install for debugging purposes
-	SetShellVarContext all
-	CopyFiles /SILENT /FILESONLY "$LOCALAPPDATA\Mullvad VPN\daemon.log" "$LOCALAPPDATA\Mullvad VPN\old-install-daemon.log"
-
-	nsExec::ExecToStack '"$SYSDIR\sc.exe" delete mullvadvpn'
-
-	# Discard return value
-	Pop $0
-	Pop $1
-
-	${If} $0 != 0
-	${AndIf} $0 != ${ERROR_SERVICE_MARKED_FOR_DELETE}
-		log::Log "Failed to delete Mullvad service: $0"
+		${If} $0 != ${MVSETUP_OK}
+		${AndIf} $0 != ${MVSETUP_DAEMON_NOT_RUNNING}
+			StrCpy $R0 "Failed to send prepare-restart to service"
+			log::LogWithDetails $R0 $1
+			Goto customRemoveFiles_abort
+		${EndIf}
 	${EndIf}
 
-	Sleep 1000
+	${StopAndDeleteService}
 
-	#
-	# Forcibly kill the service (likely marked for deletion)
-	#
+	${If} $R0 != 0
+		Goto customRemoveFiles_abort
+	${EndIf}
 
-	Var /GLOBAL DeleteService_Counter
-	Push 0
-	Pop $DeleteService_Counter
+	# Remove application files
+	log::Log "Deleting $INSTDIR"
+	RMDir /r $INSTDIR
+	IfErrors 0 customRemoveFiles_final_cleanup
 
-	customRemoveFiles_CheckServiceDeleted:
+	log::Log "Failed to remove application files"
 
-	nsExec::ExecToStack '"$SYSDIR\sc.exe" query mullvadvpn'
+	customRemoveFiles_abort:
 
-	Pop $0
-	Pop $1
+	# Break the install due to inconsistent state
+	Delete "$INSTDIR\mullvad vpn.exe"
 
-	${If} $0 != ${ERROR_SERVICE_DOES_NOT_EXIST}
-		log::Log "Attempting to forcibly kill Mullvad service"
+	# Clear firewall rules, or risk leaving persistent filters
+	${ClearFirewallRules}
 
-		nsExec::ExecToStack '"$SYSDIR\taskkill.exe" /f /fi "SERVICES eq mullvadvpn"'
-		Pop $0
-		Pop $1
-
-		# Check again whether it was deleted
-		IntOp $DeleteService_Counter $DeleteService_Counter + 1
-		${If} $DeleteService_Counter < 3
-			Sleep 1000
-			Goto customRemoveFiles_CheckServiceDeleted
-		${EndIf}
+	log::Log "Aborting uninstaller"
+	SetErrorLevel 1
+	Abort
 
-		log::Log "Failed to kill Mullvad service"
-	${EndIf}
+	customRemoveFiles_final_cleanup:
 
 	${RemoveCLIFromEnvironPath}
 
@@ -905,17 +973,22 @@
 		${ExtractWintun}
 		${RemoveWintun}
 
+		log::SetLogTarget ${LOG_VOID}
+
 		${RemoveLogsAndCache}
 		${If} $Silent != 1
 			MessageBox MB_ICONQUESTION|MB_YESNO "Would you like to remove settings files as well?" IDNO customRemoveFiles_after_remove_settings
 			${RemoveSettings}
 		${EndIf}
 		customRemoveFiles_after_remove_settings:
-	${EndIf}
+	${Else}
+		log::SetLogTarget ${LOG_VOID}
 
-	# Original removal functionality provided by Electron-builder
-	RMDir /r $INSTDIR
+		SetShellVarContext all
+		Delete "$LOCALAPPDATA\Mullvad VPN\uninstall.log"
+	${EndIf}
 
+	Pop $R0
 	Pop $1
 	Pop $0
 
diff --git a/mullvad-setup/src/main.rs b/mullvad-setup/src/main.rs
index 0e9c104db1..45186dd1a7 100644
--- a/mullvad-setup/src/main.rs
+++ b/mullvad-setup/src/main.rs
@@ -19,6 +19,16 @@ enum ExitStatus {
     Ok = 0,
     Error = 1,
     VersionNotOlder = 2,
+    DaemonNotRunning = 3,
+}
+
+impl From<Error> for ExitStatus {
+    fn from(error: Error) -> ExitStatus {
+        match error {
+            Error::RpcConnectionError(_) => ExitStatus::DaemonNotRunning,
+            _ => ExitStatus::Error,
+        }
+    }
 }
 
 #[cfg(windows)]
@@ -112,7 +122,7 @@ async fn main() {
 
     if let Err(e) = result {
         eprintln!("{}", e.display_chain());
-        process::exit(ExitStatus::Error as i32);
+        process::exit(ExitStatus::from(e) as i32);
     }
 }
 
diff --git a/windows/nsis-plugins/src/log/log.cpp b/windows/nsis-plugins/src/log/log.cpp
index 6c1fb9111a..4a7bf1bf49 100644
--- a/windows/nsis-plugins/src/log/log.cpp
+++ b/windows/nsis-plugins/src/log/log.cpp
@@ -184,17 +184,18 @@ std::wstring GetWindowsVersion()
 } // anonymous namespace
 
 //
-// Initialize
+// SetLogTarget
 //
 // Opens and maintains an open handle to the log file.
 //
 enum class LogTarget
 {
-	LOG_FILE = 0,
-	LOG_VOID
+	LOG_INSTALL = 0,
+	LOG_UNINSTALL = 1,
+	LOG_VOID = 2
 };
 
-void __declspec(dllexport) NSISCALL Initialize
+void __declspec(dllexport) NSISCALL SetLogTarget
 (
 	HWND hwndParent,
 	int string_size,
@@ -210,56 +211,66 @@ void __declspec(dllexport) NSISCALL Initialize
 	{
 		PinDll();
 
+		const wchar_t *logfile = nullptr;
+
 		int target = popint();
 		switch (target)
 		{
-			case static_cast<int>(LogTarget::LOG_FILE):
+			case static_cast<int>(LogTarget::LOG_INSTALL):
 			{
-				auto logpath = std::filesystem::path(common::fs::GetKnownFolderPath(
-					FOLDERID_ProgramData, 0, nullptr));
-
-				logpath.append(L"Mullvad VPN");
-
-				if (FALSE == CreateDirectoryW(logpath.c_str(), nullptr))
-				{
-					if (ERROR_ALREADY_EXISTS != GetLastError())
-					{
-						std::wstringstream ss;
-
-						ss << L"Cannot create folder: "
-							<< L"\""
-							<< logpath
-							<< L"\"";
-
-						THROW_ERROR(common::string::ToAnsi(ss.str()).c_str());
-					}
-				}
-
-				const auto logfile = decltype(logpath)(logpath).append(L"install.log");
-
-				g_logger = new Logger(std::make_unique<Utf8FileLogSink>(logfile, false));
-
+				logfile = L"install.log";
 				break;
-
 			}
-			case static_cast<int>(LogTarget::LOG_VOID):
+			case static_cast<int>(LogTarget::LOG_UNINSTALL):
 			{
-				g_logger = new Logger(std::make_unique<VoidLogSink>());
-
+				logfile = L"uninstall.log";
 				break;
-
+			}
+			case static_cast<int>(LogTarget::LOG_VOID):
+			{
+				delete g_logger;
+				g_logger = nullptr;
+				return;
 			}
 			default:
 			{
 				THROW_ERROR("Invalid log target");
 			}
 		}
+
+		if (nullptr == logfile)
+		{
+			THROW_ERROR("Invalid log target");
+		}
+
+		auto logpath = std::filesystem::path(common::fs::GetKnownFolderPath(
+			FOLDERID_ProgramData, 0, nullptr));
+		logpath.append(L"Mullvad VPN");
+
+		if (FALSE == CreateDirectoryW(logpath.c_str(), nullptr))
+		{
+			if (ERROR_ALREADY_EXISTS != GetLastError())
+			{
+				std::wstringstream ss;
+
+				ss << L"Cannot create folder: "
+					<< L"\""
+					<< logpath
+					<< L"\"";
+
+				THROW_ERROR(common::string::ToAnsi(ss.str()).c_str());
+			}
+		}
+
+		logpath.append(logfile);
+
+		g_logger = new Logger(std::make_unique<Utf8FileLogSink>(logpath, false));
 	}
 	catch (std::exception &err)
 	{
 		std::stringstream ss;
 
-		ss << "Failed to initialize logging plugin."
+		ss << "Failed to set logging plugin target."
 			<< std::endl
 			<< err.what();
 
diff --git a/windows/nsis-plugins/src/log/log.def b/windows/nsis-plugins/src/log/log.def
index 11cffd21e3..d51b04ab7b 100644
--- a/windows/nsis-plugins/src/log/log.def
+++ b/windows/nsis-plugins/src/log/log.def
@@ -2,7 +2,7 @@ LIBRARY log
 
 EXPORTS
 
-Initialize
+SetLogTarget
 Log
 LogWithDetails
 LogWindowsVersion