Merge branch 'windns-improve-error-handling'

4 files changed, 40 insertions, 9 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07cd008847..ba2ee5bf89 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,9 @@ Line wrap the file at 100 chars.                                              Th
 - Fix startup failure when network device with a hardware address that's not a MAC address is
   present.
 
+#### Windows
+- Improve error handling related to DNS management at the time of establishing the tunnel.
+
 ### Changed
 #### Linux
 - Increased `NetworkManager` DBus RPC timeout from 1 second to 3 seconds.
diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs
index f66263665f..0faf2b81ff 100644
--- a/mullvad-daemon/src/lib.rs
+++ b/mullvad-daemon/src/lib.rs
@@ -452,7 +452,7 @@ impl Daemon {
         } else {
             warn!("Ignoring target state change request due to shutdown");
         }
-        Self::oneshot_send(tx, Ok(()), "targe state");
+        Self::oneshot_send(tx, Ok(()), "target state");
     }
 
     fn on_get_state(&self, tx: oneshot::Sender<TunnelStateTransition>) {
diff --git a/windows/windns/src/windns/dnsagent.cpp b/windows/windns/src/windns/dnsagent.cpp
index e9b3a4246d..04bdcdb47d 100644
--- a/windows/windns/src/windns/dnsagent.cpp
+++ b/windows/windns/src/windns/dnsagent.cpp
@@ -18,12 +18,33 @@ DnsAgent::DnsAgent(Protocol protocol, INameServerSource *nameServerSource, IReco
 	, m_shutdownEvent(nullptr)
 {
 	constructNameServerUpdateEvent();
-	constructRootMonitor();
 
-	startTrackingInterfaces(discoverInterfaces());
-	updateRecoveryData();
+	try
+	{
+		constructRootMonitor();
+
+		if (false == startTrackingInterfaces(discoverInterfaces()))
+		{
+			throw std::runtime_error("Could not complete initial settings update on interfaces");
+		}
+
+		updateRecoveryData();
+
+		constructThread();
+	}
+	catch (...)
+	{
+		if (nullptr != m_shutdownEvent)
+		{
+			CloseHandle(m_shutdownEvent);
+		}
+
+		m_nameServerSource->unsubscribe(m_serverSourceEvent);
+		CloseHandle(m_serverSourceEvent);
+
+		throw;
+	}
 
-	constructThread();
 }
 
 DnsAgent::~DnsAgent()
@@ -134,7 +155,10 @@ void DnsAgent::thread()
 
 		if (WAIT_FAILED == status)
 		{
-			m_logSink->error("Failed to wait on events. Restarting wait in 1 minute.");
+			const auto error = common::error::FormatWindowsErrorPlain(GetLastError());
+			const auto message = std::string("Failed to wait on events. Restarting wait in 1 minute. Error: ").append(error);
+
+			m_logSink->error(message.c_str());
 
 			if (WAIT_OBJECT_0 == WaitForSingleObject(m_shutdownEvent, 1000 * 60))
 			{
@@ -381,17 +405,19 @@ void DnsAgent::setNameServers(const std::wstring &interfaceGuid, const std::vect
 	}
 }
 
-void DnsAgent::startTrackingInterfaces(const std::vector<std::wstring> &interfaces)
+bool DnsAgent::startTrackingInterfaces(const std::vector<std::wstring> &interfaces)
 {
 	const auto enforcedServers = m_nameServerSource->getNameServers(m_protocol);
 
+	bool successful = true;
+
 	for (const auto &iface : interfaces)
 	{
 		const auto literalOperation = std::wstring(L"Start tracking interface ").append(iface);
 
 		XTRACE(literalOperation);
 
-		ConfineOperation(common::string::ToAnsi(literalOperation).c_str(), m_logSink, [&]()
+		successful &= ConfineOperation(common::string::ToAnsi(literalOperation).c_str(), m_logSink, [&]()
 		{
 			InterfaceSnap snap(m_protocol, iface);
 
@@ -414,6 +440,8 @@ void DnsAgent::startTrackingInterfaces(const std::vector<std::wstring> &interfac
 			}
 		});
 	}
+
+	return successful;
 }
 
 void DnsAgent::stopTrackingInterfaces(const std::vector<std::wstring> &interfaces)
diff --git a/windows/windns/src/windns/dnsagent.h b/windows/windns/src/windns/dnsagent.h
index db73257330..a45cf75f00 100644
--- a/windows/windns/src/windns/dnsagent.h
+++ b/windows/windns/src/windns/dnsagent.h
@@ -76,7 +76,7 @@ private:
 
 	void setNameServers(const std::wstring &interfaceGuid, const std::vector<std::wstring> &enforcedServers);
 
-	void startTrackingInterfaces(const std::vector<std::wstring> &interfaces);
+	bool startTrackingInterfaces(const std::vector<std::wstring> &interfaces);
 	void stopTrackingInterfaces(const std::vector<std::wstring> &interfaces);
 
 	void updateRecoveryData();