nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rework Geary.Imap.ClientConnection signal and error handling

Browse source 
Remove connect, disconnect and close_error signals, since they are
implied by their respective methods completing and/or throwing an
error. Remove Deserializer pass-through signals, treat all three kinds
as generic receive errors instead.

Make Deserializer emit end-of-stream signal only on EOS, not on EOS and
on receive error, so it only signals an error condition once.
This commit is contained in:
Michael Gratton 2019-12-29 13:01:21 +10:30 • committed by Michael James Gratton
parent 7e77133bda
commit 62f1df12cb
4 changed files with 38 additions and 79 deletions
Download patch file Download diff file Expand all files Collapse all files

57
src/engine/imap/transport/imap-client-connection.vala
View file

@ -89,14 +89,6 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
private GLib.Cancellable? open_cancellable = null;
public virtual signal void connected() {
debug("Connected to %s", endpoint.to_string());
}
public virtual signal void disconnected() {
debug("Disconnected from %s", endpoint.to_string());
}
public virtual signal void sent_command(Command cmd) {
debug("SEND: %s", cmd.to_string());
}
@ -122,26 +114,14 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
warning("Received bad response: %s", err.message);
}
public virtual signal void received_eos() {
debug("Received eos");
}
public virtual signal void send_failure(Error err) {
warning("Send failure: %s", err.message);
}
public virtual signal void receive_failure(Error err) {
public virtual signal void receive_failure(GLib.Error err) {
warning("Receive failure: %s", err.message);
}
public virtual signal void deserialize_failure(Error err) {
warning("Deserialize failure: %s", err.message);
}
public virtual signal void close_error(Error err) {
warning("Close error: %s", err.message);
}
public ClientConnection(
Geary.Endpoint endpoint,
@ -216,8 +196,6 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
this.pending_queue.clear();
this.sent_queue.clear();
connected();
try {
yield open_channels_async();
} catch (Error err) {
@ -276,7 +254,6 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
close_error(close_err);
}
disconnected();
}
}
@ -359,20 +336,20 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
this.open_cancellable = new GLib.Cancellable();
// Not buffering the Deserializer because it uses a DataInputStream, which is buffered
ser_buffer = new BufferedOutputStream(ios.output_stream);
ser_buffer.set_close_base_stream(false);
// Use ClientConnection cx_id for debugging aid with Serializer/Deserializer
string id = "%04d".printf(cx_id);
ser = new Serializer(id, ser_buffer);
des = new Deserializer(id, ios.input_stream);
des.parameters_ready.connect(on_parameters_ready);
// Not buffering the Deserializer because it uses a
// DataInputStream, which is already buffered
des = new Deserializer(id, this.cx.input_stream);
des.bytes_received.connect(on_bytes_received);
des.receive_failure.connect(on_receive_failure);
des.deserialize_failure.connect(on_deserialize_failure);
des.eos.connect(on_eos);
des.end_of_stream.connect(on_eos);
des.parameters_ready.connect(on_parameters_ready);
des.receive_failure.connect(on_receive_failure);
// Start this running in the "background", it will stop when
// open_cancellable is cancelled
@ -394,11 +371,11 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
// disconnect from Deserializer before yielding to stop it
if (des != null) {
des.parameters_ready.disconnect(on_parameters_ready);
des.bytes_received.disconnect(on_bytes_received);
des.receive_failure.disconnect(on_receive_failure);
des.deserialize_failure.disconnect(on_deserialize_failure);
des.eos.disconnect(on_eos);
des.end_of_stream.disconnect(on_eos);
des.parameters_ready.disconnect(on_parameters_ready);
des.receive_failure.disconnect(on_receive_failure);
yield des.stop_async();
}
@ -586,22 +563,26 @@ public class Geary.Imap.ClientConnection : BaseObject, Logging.Source {
received_bytes(bytes);
}
private void on_eos() {
receive_failure(
new ImapError.NOT_CONNECTED(
"End of stream reading from %s", to_string()
)
);
}
private void on_receive_failure(Error err) {
receive_failure(err);
}
private void on_deserialize_failure() {
deserialize_failure(
receive_failure(
new ImapError.PARSE_ERROR(
"Unable to deserialize from %s", to_string()
)
);
}
private void on_eos() {
received_eos();
}
private void on_command_timeout(Command command) {
this.sent_queue.remove(command);
command.response_timed_out.disconnect(on_command_timeout);

24
src/engine/imap/transport/imap-client-session.vala
View file

@ -739,8 +739,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
assert(cx == null);
cx = new ClientConnection(imap_endpoint);
cx.set_logging_parent(this);
cx.connected.connect(on_network_connected);
cx.disconnected.connect(on_network_disconnected);
cx.sent_command.connect(on_network_sent_command);
cx.send_failure.connect(on_network_send_error);
cx.received_status_response.connect(on_received_status_response);
@ -748,9 +746,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
cx.received_continuation_response.connect(on_received_continuation_response);
cx.received_bytes.connect(on_received_bytes);
cx.received_bad_response.connect(on_received_bad_response);
cx.received_eos.connect(on_received_eos);
cx.receive_failure.connect(on_network_receive_failure);
cx.deserialize_failure.connect(on_network_receive_failure);
assert(connect_waiter == null);
connect_waiter = new Nonblocking.Semaphore();
@ -760,14 +756,10 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
return State.CONNECTING;
}
// this is used internally to tear-down the ClientConnection object and unhook it from
// ClientSession
private void drop_connection() {
unschedule_keepalive();
if (cx != null) {
cx.connected.disconnect(on_network_connected);
cx.disconnected.disconnect(on_network_disconnected);
cx.sent_command.disconnect(on_network_sent_command);
cx.send_failure.disconnect(on_network_send_error);
cx.received_status_response.disconnect(on_received_status_response);
@ -775,9 +767,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
cx.received_continuation_response.disconnect(on_received_continuation_response);
cx.received_bytes.disconnect(on_received_bytes);
cx.received_bad_response.disconnect(on_received_bad_response);
cx.received_eos.connect(on_received_eos);
cx.receive_failure.disconnect(on_network_receive_failure);
cx.deserialize_failure.disconnect(on_network_receive_failure);
cx = null;
}
@ -1807,14 +1797,6 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
// network connection event handlers
//
private void on_network_connected() {
fsm.issue(Event.CONNECTED);
}
private void on_network_disconnected() {
fsm.issue(Event.DISCONNECTED);
}
private void on_network_sent_command(Command cmd) {
// resechedule keepalive
schedule_keepalive();
@ -1968,11 +1950,7 @@ public class Geary.Imap.ClientSession : BaseObject, Logging.Source {
fsm.issue(Event.RECV_ERROR, null, null, err);
}
private void on_received_eos(ClientConnection cx) {
fsm.issue(Event.RECV_ERROR, null, null, null);
}
private void on_network_receive_failure(Error err) {
private void on_network_receive_failure(GLib.Error err) {
fsm.issue(Event.RECV_ERROR, null, null, err);
}

32
src/engine/imap/transport/imap-deserializer.vala
View file

@ -116,27 +116,29 @@ public class Geary.Imap.Deserializer : BaseObject {
*/
public signal void bytes_received(size_t bytes);
/**
* Fired when the underlying InputStream is closed, whether due to normal EOS or input error.
*
* @see receive_failure
*/
public signal void eos();
/**
* Fired when a syntax error has occurred.
*
* This generally means the data looks like garbage and further deserialization is unlikely
* or impossible.
* This generally means the data looks like garbage and further
* deserialization is unlikely or impossible.
*/
public signal void deserialize_failure();
/**
* Fired when an Error is trapped on the input stream.
*
* This is nonrecoverable and means the stream should be closed and this Deserializer destroyed.
* This is nonrecoverable and means the stream should be closed
* and this Deserializer destroyed.
*/
public signal void receive_failure(Error err);
public signal void receive_failure(GLib.Error err);
/**
* Fired when the underlying InputStream is closed.
*
* This is nonrecoverable and means the stream should be closed
* and this Deserializer destroyed.
*/
public signal void end_of_stream();
public Deserializer(string identifier, InputStream ins) {
@ -816,8 +818,8 @@ public class Geary.Imap.Deserializer : BaseObject {
flush_params();
// always signal as closed and notify subscribers
closed_semaphore.blind_notify();
eos();
this.closed_semaphore.blind_notify();
end_of_stream();
return State.CLOSED;
}
@ -833,9 +835,7 @@ public class Geary.Imap.Deserializer : BaseObject {
}
// always signal as closed and notify
closed_semaphore.blind_notify();
eos();
this.closed_semaphore.blind_notify();
return State.CLOSED;
}

4
test/engine/imap/transport/imap-deserializer-test.vala
View file

@ -265,7 +265,7 @@ class Geary.Imap.DeserializerTest : TestCase {
this.stream.add_data(bye.data);
bool eos = false;
this.deser.eos.connect(() => { eos = true; });
this.deser.end_of_stream.connect(() => { eos = true; });
this.process.begin(Expect.MESSAGE, (obj, ret) => { async_complete(ret); });
RootParameters? message = this.process.end(async_result());
@ -283,7 +283,7 @@ class Geary.Imap.DeserializerTest : TestCase {
this.deser.parameters_ready.connect((param) => { message = param; });
this.deser.bytes_received.connect((count) => { bytes_received += count; });
this.deser.eos.connect((param) => { eos = true; });
this.deser.end_of_stream.connect((param) => { eos = true; });
this.deser.deserialize_failure.connect(() => { deserialize_failure = true; });
this.deser.receive_failure.connect((err) => { receive_failure = true;});