GUI: Improve error logging

PythonWrapper correctly captures wxProcess stdout & stderr in sync and
async execution modes.

3 files changed, 68 insertions, 30 deletions

diff --git a/GUI/GUI/GUI/Frame.cpp b/GUI/GUI/GUI/Frame.cpp
index 0f8a3b5..7d89734 100644
--- a/GUI/GUI/GUI/Frame.cpp
+++ b/GUI/GUI/GUI/Frame.cpp
@@ -5,6 +5,7 @@
 #include <string>

 #include <vector>

 #include <wx/filepicker.h>

+#include <wx/txtstrm.h>

 

 namespace {

     enum FrameIds {

@@ -200,7 +201,7 @@ Frame::Frame()
         auto* transcribe_panel = new wxPanel(main_panel, ID_PY_PANEL);

         transcribe_panel_ = transcribe_panel;

         {

-            const auto transcribe_out_sz = wxSize(/*x_px=*/320, /*y_px=*/160);

+            const auto transcribe_out_sz = wxSize(/*x_px=*/480, /*y_px=*/160);

             auto* transcribe_out = new wxTextCtrl(transcribe_panel, ID_TRANSCRIBE_OUT,

                 wxEmptyString,

                 wxDefaultPosition,

@@ -266,7 +267,7 @@ Frame::Frame()
         auto* unity_panel = new wxPanel(main_panel, ID_UNITY_PANEL);

         unity_panel_ = unity_panel;

         {

-            const auto unity_out_sz = wxSize(/*x_px=*/320, /*y_px=*/160);

+            const auto unity_out_sz = wxSize(/*x_px=*/480, /*y_px=*/160);

             auto* unity_out = new wxTextCtrl(unity_panel, ID_UNITY_OUT,

                 wxEmptyString,

                 wxDefaultPosition,

@@ -452,11 +453,11 @@ void Frame::OnSetupPython(wxCommandEvent& event)
             transcribe_out_oss << "  Installing " << pip_dep << std::endl;

             transcribe_out_->AppendText(transcribe_out_oss.str());

         }

-        std::string transcribe_out;

-        bool res = PythonWrapper::InvokeWithArgs({ "-m", "pip", "install", pip_dep }, &transcribe_out);

+        std::string py_stdout, py_stderr;

+        bool res = PythonWrapper::InvokeWithArgs({ "-m", "pip", "install", pip_dep }, &py_stdout, &py_stderr);

         if (!res) {

             std::ostringstream transcribe_out_oss;

-            transcribe_out_oss << "Failed to install " << pip_dep << ": " << transcribe_out << std::endl;

+            transcribe_out_oss << "Failed to install " << pip_dep << ": " << py_stderr << std::endl;

             transcribe_out_->AppendText(transcribe_out_oss.str());

             return;

         }

@@ -487,9 +488,31 @@ void Frame::OnAppStart(wxCommandEvent& event) {
 	transcribe_out_->AppendText("Launching transcription engine\n");

 

     auto cb = [&](wxProcess* proc, int ret) -> void {

-		std::ostringstream transcribe_out_oss;

+        std::ostringstream transcribe_out_oss;

         transcribe_out_oss << "Transcription engine exited with code " << ret << std::endl;

 

+		wxInputStream* py_stdout = proc->GetInputStream();

+        bool first = true;

+        while (py_stdout && !py_stdout->Eof()) {

+			if (first) {

+				transcribe_out_oss << "  stdout:" << std::endl;

+                first = false;

+			}

+            wxTextInputStream iss(*py_stdout);

+            transcribe_out_oss << "    " << iss.ReadLine() << std::endl;

+        }

+

+		wxInputStream* py_stderr = proc->GetErrorStream();

+        first = true;

+        while (py_stderr && !py_stderr->Eof()) {

+			if (first) {

+				transcribe_out_oss << "  stderr:" << std::endl;

+                first = false;

+			}

+            wxTextInputStream iss(*py_stderr);

+            transcribe_out_oss << "  " << iss.ReadLine() << std::endl;

+        }

+

 		transcribe_out_->AppendText(transcribe_out_oss.str());

 		return;

     };

diff --git a/GUI/GUI/GUI/PythonWrapper.cpp b/GUI/GUI/GUI/PythonWrapper.cpp
index 71ddc4d..a0c2837 100644
--- a/GUI/GUI/GUI/PythonWrapper.cpp
+++ b/GUI/GUI/GUI/PythonWrapper.cpp
@@ -6,7 +6,9 @@
 
 class PythonProcess : public wxProcess {
 public:
-	PythonProcess(std::function<void(wxProcess* proc, int ret)>&& exit_callback) : exit_cb_(exit_callback) {}
+	PythonProcess(std::function<void(wxProcess* proc, int ret)>&& exit_callback) : exit_cb_(exit_callback) {
+		Redirect();
+	}
 
 	virtual void OnTerminate(int pid, int status) wxOVERRIDE {
 		exit_cb_(this, status);
@@ -36,58 +38,70 @@ wxProcess* PythonWrapper::InvokeAsyncWithArgs(std::vector<std::string>&& args,
 	return p;
 }
 
-bool PythonWrapper::InvokeWithArgs(std::vector<std::string>&& args, std::string* out) {
+bool PythonWrapper::InvokeWithArgs(std::vector<std::string>&& args,
+	std::string* py_stdout, std::string* py_stderr) {
 	std::ostringstream cmd_oss;
 	cmd_oss << "Resources/Python/python.exe";
 	for (const auto& arg : args) {
 		cmd_oss << " " << arg;
 	}
 
-	wxArrayString cmd_output_ary;
-	long result = wxExecute(cmd_oss.str(), cmd_output_ary);
-	std::ostringstream cmd_out_oss;
-	for (const auto& line : cmd_output_ary) {
-		if (!cmd_out_oss.str().empty()) {
-			cmd_out_oss << std::endl;
+	wxArrayString cmd_stdout;
+	wxArrayString cmd_stderr;
+	long result = wxExecute(cmd_oss.str(), cmd_stdout, cmd_stderr);
+	std::ostringstream cmd_stdout_oss;
+	for (const auto& line : cmd_stdout) {
+		if (!cmd_stdout_oss.str().empty()) {
+			cmd_stdout_oss << std::endl;
+		}
+		cmd_stdout_oss << line;
+	}
+	std::ostringstream cmd_stderr_oss;
+	for (const auto& line : cmd_stderr) {
+		if (!cmd_stderr_oss.str().empty()) {
+			cmd_stderr_oss << std::endl;
 		}
-		cmd_out_oss << line;
+		cmd_stderr_oss << line;
 	}
 	if (result == -1) {
 		std::ostringstream err_oss;
 		err_oss << "Error while executing python command \"" << cmd_oss.str() << "\": Failed to launch process";
-		*out = err_oss.str();
+		*py_stderr = err_oss.str();
 		return false;
 	} else if (result) {
 		std::ostringstream err_oss;
-		err_oss << "Error while executing python command \"" << cmd_oss.str() << "\": Process returned " << result << ": " << cmd_out_oss.str();
-		*out = err_oss.str();
+		err_oss << "Error while executing python command \"" << cmd_oss.str() <<
+			"\"" << std::endl <<
+			"Process returned " << result << ": " << std::endl <<
+			cmd_stdout_oss.str() << std::endl <<
+			cmd_stderr_oss.str() << std::endl;
+		*py_stderr = err_oss.str();
 		return false;
 	}
 
-	*out = cmd_out_oss.str();
+	*py_stdout = cmd_stdout_oss.str();
+	*py_stderr = cmd_stderr_oss.str();
 	return true;
 }
 
 
 std::string PythonWrapper::GetVersion() {
-	std::string result;
-    bool ok = InvokeWithArgs({ "--version" }, &result);
+	std::string py_stdout, py_stderr;
+    bool ok = InvokeWithArgs({ "--version" }, &py_stdout, &py_stderr);
 	if (!ok) {
-		wxLogError("Failed to get python version: %s", result.c_str());
-		result = "";
+		wxLogError("Failed to get python version: %s", py_stderr.c_str());
 	}
-	return result;
+	return py_stdout;
 }
 
 std::string PythonWrapper::DumpMics() {
-	std::string result;
+	std::string py_stdout, py_stderr;
 	const std::string dump_mics_path = "Resources/Scripts/dump_mic_devices.py";
-	bool ok = InvokeWithArgs({ dump_mics_path }, &result);
+	bool ok = InvokeWithArgs({ dump_mics_path }, &py_stdout, &py_stderr);
 	if (!ok) {
-		wxLogError("Failed to dump mic devices: %s", result.c_str());
-		result = "";
+		wxLogError("Failed to dump mic devices: %s", py_stderr.c_str());
 	}
-	return result;
+	return py_stdout;
 }
 
 bool PythonWrapper::InstallPip(std::string* out) {
diff --git a/GUI/GUI/GUI/PythonWrapper.h b/GUI/GUI/GUI/PythonWrapper.h
index f6a739e..6c90087 100644
--- a/GUI/GUI/GUI/PythonWrapper.h
+++ b/GUI/GUI/GUI/PythonWrapper.h
@@ -24,7 +24,8 @@ namespace PythonWrapper
 
 	// Invoke the interpreter with arguments.
 	// On error, sets `out` to an error message and returns false.
-	bool InvokeWithArgs(std::vector<std::string>&& args, std::string* out);
+	bool InvokeWithArgs(std::vector<std::string>&& args, std::string* py_stdout,
+		std::string* py_stderr = NULL);
 
 	// Execute python --version.
 	std::string GetVersion();