Skip to content

Commit

Permalink
Improve debug logs and error messages (#218)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryonakano authored Nov 26, 2023
1 parent 5efe4c2 commit 6c94f73
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 51 deletions.
88 changes: 52 additions & 36 deletions src/MainWindow.vala
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
*/

public class MainWindow : Gtk.ApplicationWindow {
private const string TITLE_TEXT = N_("Unable to Complete Recording");
private const string DETAIL_TEXT = N_("The following error message may be helpful:");

private Recorder recorder;
private bool destroy_on_save;

Expand Down Expand Up @@ -75,7 +72,9 @@ public class MainWindow : Gtk.ApplicationWindow {
countdown_view.countdown_ended.connect (show_record);

record_view.cancel_recording.connect (cancel_warpper);
record_view.stop_recording.connect (() => { stop_wrapper (); });
record_view.stop_recording.connect (() => {
stop_wrapper (false);
});
record_view.toggle_recording.connect ((is_recording) => {
recorder.state = is_recording ? Recorder.RecordingState.RECORDING : Recorder.RecordingState.PAUSED;
});
Expand Down Expand Up @@ -117,11 +116,17 @@ public class MainWindow : Gtk.ApplicationWindow {
});

recorder.throw_error.connect ((err, debug) => {
show_error_dialog ("%s\n%s".printf (err.message, debug));
show_error_dialog (
_("Error while recording"),
_("There was an error while recording."),
"%s\n%s".printf (err.message, debug)
);
});

recorder.save_file.connect ((tmp_full_path, suffix) => {
var tmp_file = File.new_for_path (tmp_full_path);
recorder.save_file.connect ((tmp_path, suffix) => {
debug ("recorder.save_file: tmp_path(%s), suffix(%s)", tmp_path, suffix);

var tmp_file = File.new_for_path (tmp_path);

//TRANSLATORS: This is the format of filename and %s represents a timestamp here.
//Suffix is automatically appended depending on the recording format.
Expand All @@ -132,48 +137,54 @@ public class MainWindow : Gtk.ApplicationWindow {

var autosave_dest = Application.settings.get_string ("autosave-destination");
if (autosave_dest != Application.SETTINGS_NO_AUTOSAVE) {
var final_dest = File.new_for_path (autosave_dest);
var dest = File.new_for_path (autosave_dest).get_child (final_file_name);
try {
if (tmp_file.move (final_dest.get_child (final_file_name), FileCopyFlags.OVERWRITE)) {
if (tmp_file.move (dest, FileCopyFlags.OVERWRITE)) {
welcome_view.show_success_button ();
}
} catch (Error e) {
show_error_dialog (e.message);
show_error_dialog (
_("Failed to save recording"),
_("There was an error while moving file to the designated location."),
e.message
);
recorder.remove_tmp_recording ();
}

if (destroy_on_save) {
destroy ();
}
} else {
var filechooser = new Gtk.FileDialog () {
var save_dialog = new Gtk.FileDialog () {
title = _("Save your recording"),
accept_label = _("Save"),
modal = true,
initial_name = final_file_name
};
filechooser.save.begin (this, null, (obj, res) => {
save_dialog.save.begin (this, null, (obj, res) => {
File dest;
try {
var file = filechooser.save.end (res);
if (file == null) {
return;
}

try {
if (tmp_file.move (file, FileCopyFlags.OVERWRITE)) {
welcome_view.show_success_button ();
}
} catch (Error e) {
show_error_dialog (e.message);
}
dest = save_dialog.save.end (res);
} catch (Error e) {
warning ("Failed to save recording: %s", e.message);
warning ("Failed to Gtk.FileDialog.save: %s", e.message);

// May be cancelled by user, so delete the tmp recording
try {
tmp_file.delete ();
} catch (Error e) {
show_error_dialog (e.message);
recorder.remove_tmp_recording ();

return;
}

try {
if (tmp_file.move (dest, FileCopyFlags.OVERWRITE)) {
welcome_view.show_success_button ();
}
} catch (Error e) {
show_error_dialog (
_("Failed to save recording"),
_("There was an error while moving file to the designated location."),
e.message
);
recorder.remove_tmp_recording ();
}

if (destroy_on_save) {
Expand All @@ -198,7 +209,11 @@ public class MainWindow : Gtk.ApplicationWindow {
try {
recorder.start_recording ();
} catch (Gst.ParseError e) {
show_error_dialog (e.message);
show_error_dialog (
_("Failed to start recording"),
_("There was an error while starting recording."),
e.message
);
return;
}

Expand All @@ -208,7 +223,8 @@ public class MainWindow : Gtk.ApplicationWindow {
}

private void start_wrapper () {
if (Application.settings.get_uint ("delay") != 0) {
uint delay = Application.settings.get_uint ("delay");
if (delay != 0) {
show_countdown ();
} else {
show_record ();
Expand All @@ -232,11 +248,11 @@ public class MainWindow : Gtk.ApplicationWindow {
show_welcome ();
}

private void show_error_dialog (string error_message) {
private void show_error_dialog (string primary_text, string secondary_text, string error_message) {
if (Application.IS_ON_PANTHEON) {
var error_dialog = new Granite.MessageDialog.with_image_from_icon_name (
_(TITLE_TEXT),
_(DETAIL_TEXT),
primary_text,
secondary_text,
"dialog-error", Gtk.ButtonsType.CLOSE
) {
transient_for = this,
Expand All @@ -251,9 +267,9 @@ public class MainWindow : Gtk.ApplicationWindow {
error_dialog.present ();
} else {
var error_dialog = new Gtk.AlertDialog (
_(TITLE_TEXT)
primary_text
) {
detail = _(DETAIL_TEXT) + "\n\n" + error_message,
detail = secondary_text + "\n\n" + error_message,
modal = true
};
error_dialog.show (this);
Expand Down
5 changes: 3 additions & 2 deletions src/Services/DeviceManager.vala
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public class DeviceManager : Object {

if (properties.get_boolean ("is-default", out is_default) && is_default) {
default_source = device;
debug ("Default source: \"%s\"", default_source.display_name);
}
} else if (device.has_classes ("Sink")) {
if (sinks.contains (device)) {
Expand All @@ -93,10 +94,10 @@ public class DeviceManager : Object {

if (properties.get_boolean ("is-default", out is_default) && is_default) {
default_sink = device;
debug ("Default sink: \"%s\"", default_sink.display_name);
}
} else {
// Shouldn't reach here
// NOP
assert_not_reached ();
}
}

Expand Down
25 changes: 17 additions & 8 deletions src/Services/Recorder.vala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

public class Recorder : Object {
public signal void throw_error (Error err, string debug);
public signal void save_file (string tmp_full_path, string suffix);
public signal void save_file (string tmp_path, string suffix);

private const string IGNORED_PROPNAMES[] = {
"name", "parent", "direction", "template", "caps"
Expand Down Expand Up @@ -64,7 +64,7 @@ public class Recorder : Object {
}
private double _current_peak = 0;

private string tmp_full_path;
private string tmp_path;
private string suffix;
private Gst.Pipeline pipeline;
private uint inhibit_token = 0;
Expand Down Expand Up @@ -183,9 +183,9 @@ public class Recorder : Object {
suffix = fmt_data.suffix;

string tmp_filename = "reco_" + new DateTime.now_local ().to_unix ().to_string () + suffix;
tmp_full_path = Path.build_path (Path.DIR_SEPARATOR_S, Environment.get_user_cache_dir (), tmp_filename);
sink.set ("location", tmp_full_path);
debug ("temporary saving path: %s", tmp_full_path);
tmp_path = Path.build_path (Path.DIR_SEPARATOR_S, Environment.get_user_cache_dir (), tmp_filename);
sink.set ("location", tmp_path);
debug ("temporary saving path: %s", tmp_path);

// Dual-channelization
var caps_filter = Gst.ElementFactory.make ("capsfilter", "filter");
Expand Down Expand Up @@ -243,7 +243,7 @@ public class Recorder : Object {
state = RecordingState.STOPPED;
pipeline.dispose ();

save_file (tmp_full_path, suffix);
save_file (tmp_path, suffix);
break;
case Gst.MessageType.ELEMENT:
unowned Gst.Structure? structure = msg.get_structure ();
Expand Down Expand Up @@ -273,10 +273,19 @@ public class Recorder : Object {
state = RecordingState.STOPPED;
pipeline.dispose ();

// Remove canceled file in /tmp
remove_tmp_recording ();
}

public void remove_tmp_recording () {
var tmp_file = File.new_for_path (tmp_path);
if (!tmp_file.query_exists ()) {
return;
}

try {
File.new_for_path (tmp_full_path).delete ();
tmp_file.delete ();
} catch (Error e) {
// Just failed to remove tmp file so letting user know through error dialog is not necessary
warning (e.message);
}
}
Expand Down
8 changes: 3 additions & 5 deletions src/Views/WelcomeView.vala
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public class WelcomeView : AbstractView {
});

auto_save_switch.state_set.connect ((state) => {
if (state == true) {
if (state) {
// Prevent the filechooser shown twice when enabling the autosaving
var autosave_dest = Application.settings.get_string ("autosave-destination");
if (autosave_dest != Application.SETTINGS_NO_AUTOSAVE) {
Expand All @@ -224,9 +224,7 @@ public class WelcomeView : AbstractView {
return false;
});

destination_chooser_button.clicked.connect (() => {
show_destination_chooser ();
});
destination_chooser_button.clicked.connect (show_destination_chooser);

record_button.clicked.connect (() => {
start_recording ();
Expand All @@ -244,7 +242,7 @@ public class WelcomeView : AbstractView {
auto_save_switch.active = (path != Application.SETTINGS_NO_AUTOSAVE);

var file = File.new_for_path (path);
if (file.query_exists () == false) {
if (!file.query_exists ()) {
DirUtils.create_with_parents (path, 0775);
}
}
Expand Down

0 comments on commit 6c94f73

Please sign in to comment.