Merge lp:~thomas-voss/location-service/fix-1414591 into lp:location-service/trunk

[Thomas Voß]

Make the remote::Provider::Stub fail loudly on construction if the other side is not reachable.
Relax the exception in location::Daemon::main and do not exit if instantiating a provider fails.

Make the remote::Provider::Stub fail loudly on construction if the other side is not reachable.
Relax the exception in location::Daemon::main and do not exit if instantiating a provider fails.

[Charles Kerr (charlesk)]

If we wanted to differentiate ping() errors from other errors we could use system_error with a category/code. However it's probably better like this to just log runtime errors instead of bringing the whole service down.

Looks straightforward to me.

review: Approve

[Loïc Minier (lool)]

LGTM, not tested though

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:165
http://jenkins.qa.ubuntu.com/job/location-service-ci/344/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/16
        deb: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/16/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-i386-ci/16

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/location-service-ci/344/rebuild

review: Approve (continuous-integration)

[lp:~thomas-voss/location-service/fix-1414591]

166. By Thomas Voß on 2015-01-26

Add compile guards around runtime tests.

=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
--- src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2015-01-25 12:44:20 +0000
+++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2015-01-26 20:15:51 +0000
@@ -196,6 +196,11 @@
 cul::Provider::Ptr remote::Provider::Stub::create_instance_with_config(const remote::stub::Configuration& config)
 {
     std::shared_ptr<remote::Provider::Stub> result{new remote::Provider::Stub{config}};
+
+    // This call throws if we fail to reach the remote end. With that, we make sure that
+    // we do not return a potentially invalid instance that throws later on.
+    result->ping();
+
     result->setup_event_connections();
     return result;
 }
@@ -256,6 +261,12 @@
         });
 }
 
+void remote::Provider::Stub::ping()
+{
+    // Requires reaches out to the remote side and throws in case of issues.
+    requires(Provider::Requirements::satellites);
+}
+
 remote::Provider::Stub::~Stub() noexcept
 {
     VLOG(10) << __PRETTY_FUNCTION__;

=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.h'
--- src/location_service/com/ubuntu/location/providers/remote/provider.h	2015-01-21 20:04:56 +0000
+++ src/location_service/com/ubuntu/location/providers/remote/provider.h	2015-01-26 20:15:51 +0000
@@ -83,6 +83,9 @@
 
         // Yeah, two stage init is evil.
         void setup_event_connections();
+        // ping tries to reach out to the remote end and throws
+        // if the ping fails.
+        void ping();
 
         struct Private;
         std::shared_ptr<Private> d;

=== modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp'
--- src/location_service/com/ubuntu/location/service/daemon.cpp	2015-01-25 12:45:30 +0000
+++ src/location_service/com/ubuntu/location/service/daemon.cpp	2015-01-26 20:15:51 +0000
@@ -195,8 +195,7 @@
 
         } catch(const std::runtime_error& e)
         {
-            std::cerr << "Exception instantiating provider: " << e.what() << " ... Aborting now." << std::endl;
-            return EXIT_FAILURE;
+            std::cerr << "Issue instantiating provider: " << e.what() << std::endl;
         }
     }
 

=== modified file 'src/location_service/com/ubuntu/location/service/runtime_tests.cpp'
--- src/location_service/com/ubuntu/location/service/runtime_tests.cpp	2015-01-13 13:00:24 +0000
+++ src/location_service/com/ubuntu/location/service/runtime_tests.cpp	2015-01-26 20:15:51 +0000
@@ -70,6 +70,7 @@
     }
 };
 
+#if defined(COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_GPS)
 int snr_and_ttff()
 {
     typedef boost::accumulators::accumulator_set<
@@ -170,6 +171,9 @@
 
     return 0;
 }
+#else
+int snr_and_ttff() { return 0; }
+#endif // COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_GPS
 }
 
 int location::service::execute_runtime_tests()