Merge lp:~jamesh/storage-provider-webdav/http-errors into lp:storage-provider-webdav/devel

[James Henstridge]

Improve HTTP error handling for all D-Bus methods.

Improve HTTP error handling for all D-Bus methods.

I also changed the various handler classes to store a shared_ptr<DavProvider> rather than a reference. This cleans up the memory management of the QNetworkReply objects by making sure QNetworkAccessManager is guaraneteed to live longer than the unique_ptr's we're using to store the replies.

[unity-api-1-bot (unity-api-1-bot)]

PASSED: Continuous integration, rev:27
https://jenkins.canonical.com/unity-api-1/job/lp-storage-provider-webdav-ci/32/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1100
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1107
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/898
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/898/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/898
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=zesty/898/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/898
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/898/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/898
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=zesty/898/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/898
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/898/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/898
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=zesty/898/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-provider-webdav-ci/32/rebuild

review: Approve (continuous-integration)

[Michi Henning (michihenning)]

Looks bloody awesome, thanks!

review: Approve

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2016-11-12 03:15:39 +0000
+++ src/CMakeLists.txt	2016-11-18 02:39:30 +0000
@@ -4,6 +4,7 @@
   DavDownloadJob.cpp
   DavUploadJob.cpp
   MultiStatusParser.cpp
+  http_error.cpp
   item_id.cpp
   CopyMoveHandler.cpp
   CreateFolderHandler.cpp

=== modified file 'src/CopyMoveHandler.cpp'
--- src/CopyMoveHandler.cpp	2016-11-14 06:18:18 +0000
+++ src/CopyMoveHandler.cpp	2016-11-18 02:39:30 +0000
@@ -1,11 +1,12 @@
 #include "CopyMoveHandler.h"
 #include "RetrieveMetadataHandler.h"
 #include "item_id.h"
+#include "http_error.h"
 
 using namespace std;
 using namespace unity::storage::provider;
 
-CopyMoveHandler::CopyMoveHandler(DavProvider const& provider,
+CopyMoveHandler::CopyMoveHandler(shared_ptr<DavProvider> const& provider,
                                  string const& item_id,
                                  string const& new_parent_id,
                                  string const& new_name,
@@ -15,7 +16,7 @@
       new_item_id_(make_child_id(new_parent_id, new_name, is_folder(item_id))),
       context_(ctx)
 {
-    QUrl const base_url = provider.base_url(ctx);
+    QUrl const base_url = provider->base_url(ctx);
     QNetworkRequest request(id_to_url(item_id_, base_url));
     request.setRawHeader(QByteArrayLiteral("Destination"),
                          id_to_url(new_item_id_, base_url).toEncoded());
@@ -23,7 +24,7 @@
     request.setRawHeader(QByteArrayLiteral("Overwrite"),
                          QByteArrayLiteral("F"));
 
-    reply_.reset(provider.send_request(
+    reply_.reset(provider->send_request(
         request, copy ? QByteArrayLiteral("COPY") : QByteArrayLiteral("MOVE"),
         nullptr, ctx));
     connect(reply_.get(), &QNetworkReply::finished,
@@ -43,7 +44,8 @@
 
     if (status != 201 && status != 204)
     {
-        promise_.set_exception(RemoteCommsException("Error from request: " + to_string(status)));
+        promise_.set_exception(
+            translate_http_error(reply_.get(), QByteArray(), item_id_));
         deleteLater();
         return;
     }

=== modified file 'src/CopyMoveHandler.h'
--- src/CopyMoveHandler.h	2016-11-12 03:15:39 +0000
+++ src/CopyMoveHandler.h	2016-11-18 02:39:30 +0000
@@ -13,7 +13,7 @@
 class CopyMoveHandler : public QObject {
     Q_OBJECT
 public:
-    CopyMoveHandler(DavProvider const& provider,
+    CopyMoveHandler(std::shared_ptr<DavProvider> const& provider,
                     std::string const& item_id,
                     std::string const& new_parent_id,
                     std::string const& new_name,
@@ -29,7 +29,7 @@
 private:
     boost::promise<unity::storage::provider::Item> promise_;
 
-    DavProvider const& provider_;
+    std::shared_ptr<DavProvider> const provider_;
     std::string const item_id_;
     std::string const new_item_id_;
     unity::storage::provider::Context const context_;

=== modified file 'src/CreateFolderHandler.cpp'
--- src/CreateFolderHandler.cpp	2016-11-12 03:15:39 +0000
+++ src/CreateFolderHandler.cpp	2016-11-18 02:39:30 +0000
@@ -1,22 +1,23 @@
 #include "CreateFolderHandler.h"
 #include "RetrieveMetadataHandler.h"
 #include "item_id.h"
+#include "http_error.h"
 
 using namespace std;
 using namespace unity::storage::provider;
 
-CreateFolderHandler::CreateFolderHandler(DavProvider const& provider,
+CreateFolderHandler::CreateFolderHandler(shared_ptr<DavProvider> const& provider,
                                          string const& parent_id,
                                          string const& name,
                                          Context const& ctx)
     : provider_(provider), item_id_(make_child_id(parent_id, name, true)),
       context_(ctx)
 {
-    QUrl const base_url = provider.base_url(ctx);
+    QUrl const base_url = provider->base_url(ctx);
     QNetworkRequest request(id_to_url(item_id_, base_url));
-    mkcol_.reset(provider.send_request(request, QByteArrayLiteral("MKCOL"),
-                                       nullptr, ctx));
-    connect(mkcol_.get(), &QNetworkReply::finished,
+    reply_.reset(provider->send_request(request, QByteArrayLiteral("MKCOL"),
+                                        nullptr, ctx));
+    connect(reply_.get(), &QNetworkReply::finished,
             this, &CreateFolderHandler::onFinished);
 }
 
@@ -29,11 +30,12 @@
 
 void CreateFolderHandler::onFinished()
 {
-    auto status = mkcol_->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    auto status = reply_->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
     if (status != 201)
     {
-        promise_.set_exception(RemoteCommsException("Error from MKCOL: " + to_string(status)));
+        promise_.set_exception(
+            translate_http_error(reply_.get(), QByteArray(), item_id_));
         deleteLater();
         return;
     }

=== modified file 'src/CreateFolderHandler.h'
--- src/CreateFolderHandler.h	2016-11-10 09:41:35 +0000
+++ src/CreateFolderHandler.h	2016-11-18 02:39:30 +0000
@@ -13,7 +13,7 @@
 class CreateFolderHandler : public QObject {
     Q_OBJECT
 public:
-    CreateFolderHandler(DavProvider const& provider,
+    CreateFolderHandler(std::shared_ptr<DavProvider> const& provider,
                         std::string const& parent_id, std::string const& name,
                         unity::storage::provider::Context const& ctx);
     ~CreateFolderHandler();
@@ -26,10 +26,10 @@
 private:
     boost::promise<unity::storage::provider::Item> promise_;
 
-    DavProvider const& provider_;
+    std::shared_ptr<DavProvider> const provider_;
     std::string const item_id_;
     unity::storage::provider::Context const context_;
 
-    std::unique_ptr<QNetworkReply> mkcol_;
+    std::unique_ptr<QNetworkReply> reply_;
     std::unique_ptr<RetrieveMetadataHandler> metadata_;
 };

=== modified file 'src/DavDownloadJob.cpp'
--- src/DavDownloadJob.cpp	2016-11-15 11:32:58 +0000
+++ src/DavDownloadJob.cpp	2016-11-18 02:39:30 +0000
@@ -2,6 +2,7 @@
 #include "DavProvider.h"
 #include "RetrieveMetadataHandler.h"
 #include "item_id.h"
+#include "http_error.h"
 
 #include <unity/storage/provider/Exceptions.h>
 
@@ -27,14 +28,14 @@
 
 }
 
-DavDownloadJob::DavDownloadJob(DavProvider const& provider,
+DavDownloadJob::DavDownloadJob(shared_ptr<DavProvider> const& provider,
                                string const& item_id,
                                string const& match_etag,
                                Context const& ctx)
     : QObject(), DownloadJob(make_download_id()), provider_(provider),
       item_id_(item_id)
 {
-    QUrl base_url = provider.base_url(ctx);
+    QUrl base_url = provider->base_url(ctx);
     QNetworkRequest request(id_to_url(item_id, base_url));
     if (!match_etag.empty())
     {
@@ -42,15 +43,15 @@
                              QByteArray::fromStdString(match_etag));
     }
 
-    reply_ = provider.send_request(
-        request, QByteArrayLiteral("GET"), nullptr, ctx);
-    assert(reply_ != nullptr);
+    reply_.reset(provider->send_request(
+        request, QByteArrayLiteral("GET"), nullptr, ctx));
+    assert(reply_.get() != nullptr);
     reply_->setReadBufferSize(CHUNK_SIZE);
-    connect(reply_, &QNetworkReply::finished,
+    connect(reply_.get(), &QNetworkReply::finished,
             this, &DavDownloadJob::onReplyFinished);
-    connect(reply_, &QIODevice::readyRead,
+    connect(reply_.get(), &QIODevice::readyRead,
             this, &DavDownloadJob::onReplyReadyRead);
-    connect(reply_, &QIODevice::readChannelFinished,
+    connect(reply_.get(), &QIODevice::readChannelFinished,
             this, &DavDownloadJob::onReplyReadChannelFinished);
     writer_.setSocketDescriptor(
         dup(write_socket()), QLocalSocket::ConnectedState, QIODevice::WriteOnly);
@@ -58,50 +59,56 @@
             this, &DavDownloadJob::onSocketBytesWritten);
 }
 
-DavDownloadJob::~DavDownloadJob()
-{
-    if (!reply_.isNull())
-    {
-        reply_->deleteLater();
-    }
-}
+DavDownloadJob::~DavDownloadJob() = default;
 
 void DavDownloadJob::onReplyFinished()
 {
-    // If we haven't seen HTTP response headers and are in an error
-    // state, report that.
-    if (!seen_header_ && reply_->error() != QNetworkReply::NoError)
+    if (!seen_header_ || is_error_)
     {
-        handle_error(RemoteCommsException("Error connecting to server: " +
-                                          reply_->errorString().toStdString()));
+        try
+        {
+            boost::rethrow_exception(
+                translate_http_error(reply_.get(), error_body_, item_id_));
+        }
+        catch (...)
+        {
+            handle_error(std::current_exception());
+        }
     }
 }
 
 void DavDownloadJob::onReplyReadyRead()
 {
-    if (error_)
-    {
-        return;
-    }
-
     if (!seen_header_)
     {
         seen_header_ = true;
         auto status = reply_->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         if (status != 200)
         {
-            handle_error(RemoteCommsException("Unexpected status code: " +
-                                              to_string(status)));
-            return;
+            is_error_ = true;
         }
     }
 
-    maybe_send_chunk();
+    if (is_error_)
+    {
+        if (error_body_.size() < MAX_ERROR_BODY_LENGTH)
+        {
+            error_body_.append(reply_->readAll());
+        }
+        else
+        {
+            reply_->close();
+        }
+    }
+    else
+    {
+        maybe_send_chunk();
+    }
 }
 
 void DavDownloadJob::onReplyReadChannelFinished()
 {
-    if (error_)
+    if (is_error_)
     {
         return;
     }
@@ -112,7 +119,7 @@
 
 void DavDownloadJob::onSocketBytesWritten(int64_t bytes)
 {
-    if (error_)
+    if (is_error_)
     {
         return;
     }
@@ -146,7 +153,8 @@
     int n_read = reply_->read(buffer, CHUNK_SIZE);
     if (n_read < 0)
     {
-        handle_error(RemoteCommsException("Failed to read from server"));
+        handle_error(RemoteCommsException("Failed to read from server: " +
+                                          reply_->errorString().toStdString()));
         return;
     }
     bytes_read_ += n_read;
@@ -154,17 +162,24 @@
     int n_written = writer_.write(buffer, n_read);
     if (n_written < 0)
     {
-        handle_error(ResourceException("Error writing to socket", 0));
+        handle_error(ResourceException(
+                         "Error writing to socket: "
+                         + writer_.errorString().toStdString(), 0));
         return;
     }
 }
 
 void DavDownloadJob::handle_error(StorageException const& exc)
 {
-    error_ = true;
+    handle_error(std::make_exception_ptr(exc));
+}
+
+void DavDownloadJob::handle_error(std::exception_ptr ep)
+{
+    is_error_ = true;
     reply_->close();
     writer_.close();
-    report_error(std::make_exception_ptr(exc));
+    report_error(ep);
 }
 
 

=== modified file 'src/DavDownloadJob.h'
--- src/DavDownloadJob.h	2016-11-09 11:48:39 +0000
+++ src/DavDownloadJob.h	2016-11-18 02:39:30 +0000
@@ -1,9 +1,9 @@
 #pragma once
 
+#include <QByteArray>
 #include <QLocalSocket>
 #include <QNetworkReply>
 #include <QObject>
-#include <QPointer>
 #include <QUrl>
 #include <unity/storage/provider/Exceptions.h>
 #include <unity/storage/provider/ProviderBase.h>
@@ -19,8 +19,8 @@
 {
     Q_OBJECT
 public:
-    DavDownloadJob(DavProvider const& provider, std::string const& item_id,
-                   std::string const& match_etag,
+    DavDownloadJob(std::shared_ptr<DavProvider> const& provider,
+                   std::string const& item_id, std::string const& match_etag,
                    unity::storage::provider::Context const& ctx);
     ~DavDownloadJob();
 
@@ -37,15 +37,18 @@
 private:
     void maybe_send_chunk();
     void handle_error(unity::storage::provider::StorageException const& exc);
+    void handle_error(std::exception_ptr ep);
 
-    DavProvider const& provider_;
+    std::shared_ptr<DavProvider> const provider_;
     std::string const item_id_;
     QLocalSocket writer_;
-    QPointer<QNetworkReply> reply_;
+    std::unique_ptr<QNetworkReply> reply_;
 
     bool seen_header_ = false;
-    bool error_ = false;
     bool read_channel_finished_ = false;
     int64_t bytes_read_ = 0;
     int64_t bytes_written_ = 0;
+
+    bool is_error_ = false;
+    QByteArray error_body_;
 };

=== modified file 'src/DavProvider.cpp'
--- src/DavProvider.cpp	2016-11-16 02:58:18 +0000
+++ src/DavProvider.cpp	2016-11-18 02:39:30 +0000
@@ -31,11 +31,16 @@
 
 DavProvider::~DavProvider() = default;
 
+std::shared_ptr<DavProvider> DavProvider::shared_from_this()
+{
+    return static_pointer_cast<DavProvider>(ProviderBase::shared_from_this());
+}
+
 boost::future<ItemList> DavProvider::roots(
     vector<string> const& metadata_keys, Context const& ctx)
 {
     Q_UNUSED(metadata_keys);
-    auto handler = new RootsHandler(*this, ctx);
+    auto handler = new RootsHandler(shared_from_this(), ctx);
     return handler->get_future();
 }
 
@@ -48,7 +53,7 @@
     {
         throw InvalidArgumentException("Invalid paging token: " + page_token);
     }
-    auto handler = new ListHandler(*this, item_id, ctx);
+    auto handler = new ListHandler(shared_from_this(), item_id, ctx);
     return handler->get_future();
 }
 
@@ -58,7 +63,7 @@
 {
     Q_UNUSED(metadata_keys);
     string item_id = make_child_id(parent_id, name);
-    auto handler = new LookupHandler(*this, item_id, ctx);
+    auto handler = new LookupHandler(shared_from_this(), item_id, ctx);
     return handler->get_future();
 }
 
@@ -67,7 +72,7 @@
     Context const& ctx)
 {
     Q_UNUSED(metadata_keys);
-    auto handler = new MetadataHandler(*this, item_id, ctx);
+    auto handler = new MetadataHandler(shared_from_this(), item_id, ctx);
     return handler->get_future();
 }
 
@@ -76,7 +81,8 @@
     vector<string> const& metadata_keys, Context const& ctx)
 {
     Q_UNUSED(metadata_keys);
-    auto handler = new CreateFolderHandler(*this, parent_id, name, ctx);
+    auto handler = new CreateFolderHandler(
+        shared_from_this(), parent_id, name, ctx);
     return handler->get_future();
 }
 
@@ -89,7 +95,8 @@
     string item_id = make_child_id(parent_id, name);
     boost::promise<unique_ptr<UploadJob>> p;
     p.set_value(unique_ptr<UploadJob>(new DavUploadJob(
-        *this, item_id, size, content_type, allow_overwrite, string(), ctx)));
+        shared_from_this(), item_id, size, content_type, allow_overwrite,
+        string(), ctx)));
     return p.get_future();
 }
 
@@ -100,7 +107,7 @@
     Q_UNUSED(metadata_keys);
     boost::promise<unique_ptr<UploadJob>> p;
     p.set_value(unique_ptr<UploadJob>(new DavUploadJob(
-        *this, item_id, size, string(), true, old_etag, ctx)));
+        shared_from_this(), item_id, size, string(), true, old_etag, ctx)));
     return p.get_future();
 }
 
@@ -109,14 +116,14 @@
 {
     boost::promise<unique_ptr<DownloadJob>> p;
     p.set_value(unique_ptr<DownloadJob>(new DavDownloadJob(
-        *this, item_id, match_etag, ctx)));
+        shared_from_this(), item_id, match_etag, ctx)));
     return p.get_future();
 }
 
 boost::future<void> DavProvider::delete_item(
     string const& item_id, Context const& ctx)
 {
-    auto handler = new DeleteHandler(*this, item_id, ctx);
+    auto handler = new DeleteHandler(shared_from_this(), item_id, ctx);
     return handler->get_future();
 }
 
@@ -125,8 +132,8 @@
     vector<string> const& metadata_keys, Context const& ctx)
 {
     Q_UNUSED(metadata_keys);
-    auto handler = new CopyMoveHandler(*this, item_id, new_parent_id,
-                                       new_name, false, ctx);
+    auto handler = new CopyMoveHandler(
+        shared_from_this(), item_id, new_parent_id, new_name, false, ctx);
     return handler->get_future();
 }
 
@@ -135,8 +142,8 @@
     vector<string> const& metadata_keys, Context const& ctx)
 {
     Q_UNUSED(metadata_keys);
-    auto handler = new CopyMoveHandler(*this, item_id, new_parent_id,
-                                       new_name, true, ctx);
+    auto handler = new CopyMoveHandler(
+        shared_from_this(), item_id, new_parent_id, new_name, true, ctx);
     return handler->get_future();
 }
 

=== modified file 'src/DavProvider.h'
--- src/DavProvider.h	2016-11-15 11:32:58 +0000
+++ src/DavProvider.h	2016-11-18 02:39:30 +0000
@@ -78,4 +78,7 @@
 
 protected:
     std::unique_ptr<QNetworkAccessManager> const network_;
+
+private:
+    inline std::shared_ptr<DavProvider> shared_from_this();
 };

=== modified file 'src/DavUploadJob.cpp'
--- src/DavUploadJob.cpp	2016-11-15 11:32:58 +0000
+++ src/DavUploadJob.cpp	2016-11-18 02:39:30 +0000
@@ -2,6 +2,7 @@
 #include "DavProvider.h"
 #include "RetrieveMetadataHandler.h"
 #include "item_id.h"
+#include "http_error.h"
 
 #include <unity/storage/provider/Exceptions.h>
 
@@ -25,12 +26,12 @@
 
 }
 
-DavUploadJob::DavUploadJob(DavProvider const& provider, string const& item_id,
-                           int64_t size, string const& content_type,
-                           bool allow_overwrite, string const& old_etag,
-                           Context const& ctx)
+DavUploadJob::DavUploadJob(shared_ptr<DavProvider> const& provider,
+                           string const& item_id, int64_t size,
+                           string const& content_type, bool allow_overwrite,
+                           string const& old_etag, Context const& ctx)
     : QObject(), UploadJob(make_upload_id()), provider_(provider),
-      item_id_(item_id), base_url_(provider.base_url(ctx)), size_(size),
+      item_id_(item_id), base_url_(provider->base_url(ctx)), size_(size),
       context_(ctx)
 {
     QNetworkRequest request(id_to_url(item_id, base_url_));
@@ -54,27 +55,15 @@
 
     reader_.setSocketDescriptor(
         dup(read_socket()), QLocalSocket::ConnectedState, QIODevice::ReadOnly);
-    reply_.reset(provider.send_request(
+    reply_.reset(provider->send_request(
         request, QByteArrayLiteral("PUT"), &reader_, ctx));
     assert(reply_.get() != nullptr);
-    connect(reply_.get(), static_cast<void(QNetworkReply::*)(QNetworkReply::NetworkError)>(&QNetworkReply::error),
-            this, &DavUploadJob::onReplyError);
     connect(reply_.get(), &QNetworkReply::finished,
             this, &DavUploadJob::onReplyFinished);
 }
 
 DavUploadJob::~DavUploadJob() = default;
 
-void DavUploadJob::onReplyError(QNetworkReply::NetworkError code)
-{
-    if (promise_set_)
-    {
-        return;
-    }
-    promise_.set_exception(RemoteCommsException("Error from QNetworkReply: " + to_string(code)));
-    promise_set_ = true;
-}
-
 void DavUploadJob::onReplyFinished()
 {
     if (promise_set_)
@@ -85,7 +74,8 @@
     // Is this a success status code?
     if (status / 100 != 2)
     {
-        promise_.set_exception(RemoteCommsException("Error from PUT: " + to_string(status)));
+        promise_.set_exception(
+            translate_http_error(reply_.get(), QByteArray(), item_id_));
         promise_set_ = true;
         return;
     }

=== modified file 'src/DavUploadJob.h'
--- src/DavUploadJob.h	2016-11-01 02:53:02 +0000
+++ src/DavUploadJob.h	2016-11-18 02:39:30 +0000
@@ -18,9 +18,10 @@
 {
     Q_OBJECT
 public:
-    DavUploadJob(DavProvider const& provider, std::string const& item_id,
-                 int64_t size, std::string const& content_type,
-                 bool allow_overwrite, std::string const& old_etag,
+    DavUploadJob(std::shared_ptr<DavProvider> const& provider,
+                 std::string const& item_id, int64_t size,
+                 std::string const& content_type, bool allow_overwrite,
+                 std::string const& old_etag,
                  unity::storage::provider::Context const& ctx);
     ~DavUploadJob();
 
@@ -28,11 +29,10 @@
     boost::future<unity::storage::provider::Item> finish() override;
 
 private Q_SLOTS:
-    void onReplyError(QNetworkReply::NetworkError code);
     void onReplyFinished();
 
 private:
-    DavProvider const& provider_;
+    std::shared_ptr<DavProvider> const provider_;
     std::string const item_id_;
     QUrl const base_url_;
     int64_t const size_;

=== modified file 'src/DeleteHandler.cpp'
--- src/DeleteHandler.cpp	2016-11-11 07:06:07 +0000
+++ src/DeleteHandler.cpp	2016-11-18 02:39:30 +0000
@@ -1,22 +1,20 @@
 #include "DeleteHandler.h"
-
-#include <unity/storage/provider/Exceptions.h>
-
 #include "DavProvider.h"
 #include "item_id.h"
+#include "http_error.h"
 
 using namespace std;
 using namespace unity::storage::provider;
 
-DeleteHandler::DeleteHandler(DavProvider const& provider,
+DeleteHandler::DeleteHandler(shared_ptr<DavProvider> const& provider,
                              string const& item_id,
                              Context const& ctx)
     : provider_(provider), item_id_(item_id)
 {
-    QUrl const base_url = provider.base_url(ctx);
+    QUrl const base_url = provider->base_url(ctx);
     QNetworkRequest request(id_to_url(item_id_, base_url));
-    reply_.reset(provider.send_request(request, QByteArrayLiteral("DELETE"),
-                                       nullptr, ctx));
+    reply_.reset(provider->send_request(request, QByteArrayLiteral("DELETE"),
+                                        nullptr, ctx));
     connect(reply_.get(), &QNetworkReply::finished,
             this, &DeleteHandler::onFinished);
 }
@@ -38,7 +36,8 @@
     }
     else
     {
-        promise_.set_exception(RemoteCommsException("Error from DELETE: " + to_string(status)));
+        promise_.set_exception(
+            translate_http_error(reply_.get(), QByteArray(), item_id_));
     }
 
     deleteLater();

=== modified file 'src/DeleteHandler.h'
--- src/DeleteHandler.h	2016-11-11 03:53:48 +0000
+++ src/DeleteHandler.h	2016-11-18 02:39:30 +0000
@@ -12,7 +12,7 @@
 class DeleteHandler : public QObject {
     Q_OBJECT
 public:
-    DeleteHandler(DavProvider const& provider, std::string const& item_id,
+    DeleteHandler(std::shared_ptr<DavProvider> const& provider, std::string const& item_id,
                   unity::storage::provider::Context const& ctx);
     ~DeleteHandler();
 
@@ -24,7 +24,7 @@
 private:
     boost::promise<void> promise_;
 
-    DavProvider const& provider_;
+    std::shared_ptr<DavProvider> const provider_;
     std::string const item_id_;
 
     std::unique_ptr<QNetworkReply> reply_;

=== modified file 'src/ListHandler.cpp'
--- src/ListHandler.cpp	2016-10-31 01:40:28 +0000
+++ src/ListHandler.cpp	2016-11-18 02:39:30 +0000
@@ -8,9 +8,9 @@
 using namespace std;
 using namespace unity::storage::provider;
 
-ListHandler::ListHandler(DavProvider const& provider,
+ListHandler::ListHandler(std::shared_ptr<DavProvider> const& provider,
                          string const& parent_id, Context const& ctx)
-    : PropFindHandler(provider, parent_id, 1, ctx), parent_id_(parent_id)
+    : PropFindHandler(provider, parent_id, 1, ctx)
 {
 }
 
@@ -34,7 +34,7 @@
     // itself, so remove it from the list.
     items_.erase(remove_if(items_.begin(), items_.end(),
                            [&](Item const& item) -> bool {
-                               return item.item_id == parent_id_;
+                               return item.item_id == item_id_;
                            }), items_.end());
 
     promise_.set_value(make_tuple(move(items_), string()));

=== modified file 'src/ListHandler.h'
--- src/ListHandler.h	2016-10-31 07:32:53 +0000
+++ src/ListHandler.h	2016-11-18 02:39:30 +0000
@@ -10,7 +10,7 @@
 class ListHandler : public PropFindHandler {
     Q_OBJECT
 public:
-    ListHandler(DavProvider const& provider,
+    ListHandler(std::shared_ptr<DavProvider> const& provider,
                 std::string const& parent_id,
                 unity::storage::provider::Context const& ctx);
     ~ListHandler();
@@ -18,7 +18,6 @@
     boost::future<std::tuple<unity::storage::provider::ItemList,std::string>> get_future();
 
 private:
-    std::string const parent_id_;
     boost::promise<std::tuple<unity::storage::provider::ItemList,std::string>> promise_;
 
 protected:

=== modified file 'src/LookupHandler.cpp'
--- src/LookupHandler.cpp	2016-09-28 01:30:31 +0000
+++ src/LookupHandler.cpp	2016-11-18 02:39:30 +0000
@@ -5,7 +5,7 @@
 using namespace std;
 using namespace unity::storage::provider;
 
-LookupHandler::LookupHandler(DavProvider const& provider,
+LookupHandler::LookupHandler(std::shared_ptr<DavProvider> const& provider,
                              string const& item_id, Context const& ctx)
     : PropFindHandler(provider, item_id, 0, ctx)
 {

=== modified file 'src/LookupHandler.h'
--- src/LookupHandler.h	2016-10-31 07:32:53 +0000
+++ src/LookupHandler.h	2016-11-18 02:39:30 +0000
@@ -9,7 +9,7 @@
 class LookupHandler : public PropFindHandler {
     Q_OBJECT
 public:
-    LookupHandler(DavProvider const& provider,
+    LookupHandler(std::shared_ptr<DavProvider> const& provider,
                   std::string const& item_id,
                   unity::storage::provider::Context const& ctx);
     ~LookupHandler();

=== modified file 'src/MetadataHandler.cpp'
--- src/MetadataHandler.cpp	2016-10-31 01:40:28 +0000
+++ src/MetadataHandler.cpp	2016-11-18 02:39:30 +0000
@@ -5,9 +5,9 @@
 using namespace std;
 using namespace unity::storage::provider;
 
-MetadataHandler::MetadataHandler(DavProvider const& provider,
+MetadataHandler::MetadataHandler(shared_ptr<DavProvider> const& provider,
                                  string const& item_id, Context const& ctx)
-    : PropFindHandler(provider, item_id, 0, ctx), item_id_(item_id)
+    : PropFindHandler(provider, item_id, 0, ctx)
 {
 }
 

=== modified file 'src/MetadataHandler.h'
--- src/MetadataHandler.h	2016-10-31 07:32:53 +0000
+++ src/MetadataHandler.h	2016-11-18 02:39:30 +0000
@@ -9,7 +9,7 @@
 class MetadataHandler : public PropFindHandler {
     Q_OBJECT
 public:
-    MetadataHandler(DavProvider const& provider,
+    MetadataHandler(std::shared_ptr<DavProvider> const& provider,
                     std::string const& item_id,
                     unity::storage::provider::Context const& ctx);
     ~MetadataHandler();
@@ -17,7 +17,6 @@
     boost::future<unity::storage::provider::Item> get_future();
 
 private:
-    std::string const item_id_;
     boost::promise<unity::storage::provider::Item> promise_;
 
 protected:

=== modified file 'src/PropFindHandler.cpp'
--- src/PropFindHandler.cpp	2016-10-31 07:32:08 +0000
+++ src/PropFindHandler.cpp	2016-11-18 02:39:30 +0000
@@ -1,5 +1,6 @@
 #include "PropFindHandler.h"
 #include "item_id.h"
+#include "http_error.h"
 
 #include <cassert>
 
@@ -21,10 +22,12 @@
 </D:propfind>)");
 }
 
-PropFindHandler::PropFindHandler(DavProvider const& provider, string const& item_id, int depth, Context const& ctx)
-    : provider_(provider), base_url_(provider.base_url(ctx))
+PropFindHandler::PropFindHandler(shared_ptr<DavProvider> const& provider,
+                                 string const& item_id, int depth,
+                                 Context const& ctx)
+    : provider_(provider), base_url_(provider->base_url(ctx)), item_id_(item_id)
 {
-    QNetworkRequest request(id_to_url(item_id, base_url_));
+    QNetworkRequest request(id_to_url(item_id_, base_url_));
     request.setRawHeader(QByteArrayLiteral("Depth"), QByteArray::number(depth));
     request.setHeader(QNetworkRequest::ContentTypeHeader,
                       QStringLiteral("application/xml; charset=\"utf-8\""));
@@ -32,14 +35,14 @@
     request_body_.setData(PROPFIND_BODY);
     request_body_.open(QIODevice::ReadOnly);
 
-    reply_.reset(provider.send_request(request, QByteArrayLiteral("PROPFIND"),
-                                       &request_body_, ctx));
+    reply_.reset(provider->send_request(request, QByteArrayLiteral("PROPFIND"),
+                                        &request_body_, ctx));
     assert(reply_.get() != nullptr);
 
-    connect(reply_.get(), static_cast<void(QNetworkReply::*)(QNetworkReply::NetworkError)>(&QNetworkReply::error),
-            this, &PropFindHandler::onError);
     connect(reply_.get(), &QIODevice::readyRead,
-            this, &PropFindHandler::onReadyRead);
+            this, &PropFindHandler::onReplyReadyRead);
+    connect(reply_.get(), &QNetworkReply::finished,
+            this, &PropFindHandler::onReplyFinished);
 }
 
 PropFindHandler::~PropFindHandler()
@@ -58,12 +61,17 @@
 
 void PropFindHandler::reportError(StorageException const& error)
 {
+    reportError(boost::copy_exception(error));
+}
+
+void PropFindHandler::reportError(boost::exception_ptr const& ep)
+{
     if (finished_)
     {
         return;
     }
 
-    error_ = boost::copy_exception(error);
+    error_ = ep;
     finished_ = true;
     finish();
 }
@@ -79,28 +87,46 @@
     finish();
 }
 
-void PropFindHandler::onError(QNetworkReply::NetworkError code)
+void PropFindHandler::onReplyReadyRead()
 {
-    reportError(RemoteCommsException("Error from QNetworkReply: " +
-                                     to_string(code)));
+    if (!seen_headers_)
+    {
+        seen_headers_ = true;
+        auto status = reply_->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+        if (status == 207)
+        {
+            disconnect(reply_.get(), &QIODevice::readyRead,
+                       this, &PropFindHandler::onReplyReadyRead);
+            parser_.reset(new MultiStatusParser(reply_->request().url(), reply_.get()));
+            connect(parser_.get(), &MultiStatusParser::response,
+                    this, &PropFindHandler::onParserResponse);
+            connect(parser_.get(), &MultiStatusParser::finished,
+                    this, &PropFindHandler::onParserFinished);
+        }
+        else
+        {
+            is_error_ = true;
+        }
+    }
+    if (is_error_)
+    {
+        if (error_body_.size() < MAX_ERROR_BODY_LENGTH)
+        {
+            error_body_.append(reply_->readAll());
+        }
+        else
+        {
+            reply_->close();
+        }
+    }
 }
 
-void PropFindHandler::onReadyRead()
+void PropFindHandler::onReplyFinished()
 {
-    disconnect(reply_.get(), &QIODevice::readyRead,
-               this, &PropFindHandler::onReadyRead);
-    auto status = reply_->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
-
-    if (status != 207)
+    if (!seen_headers_ || is_error_)
     {
-        reportError(RemoteCommsException("Expected 207 response, but got " + to_string(status)));
-        return;
+        reportError(translate_http_error(reply_.get(), error_body_, item_id_));
     }
-    parser_.reset(new MultiStatusParser(reply_->request().url(), reply_.get()));
-    connect(parser_.get(), &MultiStatusParser::response,
-            this, &PropFindHandler::onParserResponse);
-    connect(parser_.get(), &MultiStatusParser::finished,
-            this, &PropFindHandler::onParserFinished);
 }
 
 void PropFindHandler::onParserResponse(QUrl const& href, vector<MultiStatusProperty> const& properties, int status)
@@ -112,7 +138,7 @@
     }
     try
     {
-        Item item = provider_.make_item(href, base_url_, properties);
+        Item item = provider_->make_item(href, base_url_, properties);
         items_.emplace_back(move(item));
     }
     catch (StorageException const& error)

=== modified file 'src/PropFindHandler.h'
--- src/PropFindHandler.h	2016-10-31 07:32:08 +0000
+++ src/PropFindHandler.h	2016-11-18 02:39:30 +0000
@@ -13,7 +13,7 @@
 class PropFindHandler : public QObject {
     Q_OBJECT
 public:
-    PropFindHandler(DavProvider const& provider,
+    PropFindHandler(std::shared_ptr<DavProvider> const& provider,
                     std::string const& item_id, int depth,
                     unity::storage::provider::Context const& ctx);
     ~PropFindHandler();
@@ -22,8 +22,8 @@
 
 private Q_SLOTS:
     // From QNetworkReply
-    void onError(QNetworkReply::NetworkError code);
-    void onReadyRead();
+    void onReplyReadyRead();
+    void onReplyFinished();
 
     // From MultiStatusParser
     void onParserResponse(QUrl const& href, std::vector<MultiStatusProperty> const& properties, int status);
@@ -31,20 +31,25 @@
 
 private:
     void reportError(unity::storage::provider::StorageException const& error);
+    void reportError(boost::exception_ptr const& ep);
     void reportSuccess();
 
+    bool seen_headers_ = false;
+    bool is_error_ = false;
     bool finished_ = false;
     boost::promise<unity::storage::provider::ItemList> promise_;
 
-    DavProvider const& provider_;
+    std::shared_ptr<DavProvider> const provider_;
     QUrl base_url_;
     QBuffer request_body_;
     std::unique_ptr<QNetworkReply> reply_;
     std::unique_ptr<MultiStatusParser> parser_;
+    QByteArray error_body_;
 
 protected:
     virtual void finish() = 0;
 
+    std::string const item_id_;
     unity::storage::provider::ItemList items_;
     boost::exception_ptr error_;
 };

=== modified file 'src/RetrieveMetadataHandler.cpp'
--- src/RetrieveMetadataHandler.cpp	2016-11-01 02:53:02 +0000
+++ src/RetrieveMetadataHandler.cpp	2016-11-18 02:39:30 +0000
@@ -7,12 +7,11 @@
 using namespace std;
 using namespace unity::storage::provider;
 
-RetrieveMetadataHandler::RetrieveMetadataHandler(DavProvider const& provider,
+RetrieveMetadataHandler::RetrieveMetadataHandler(shared_ptr<DavProvider> const& provider,
                                                  string const& item_id,
                                                  Context const& ctx,
                                                  Callback callback)
-    : PropFindHandler(provider, item_id, 0, ctx), item_id_(item_id),
-      callback_(callback)
+    : PropFindHandler(provider, item_id, 0, ctx), callback_(callback)
 {
 }
 
@@ -22,7 +21,7 @@
 {
     Item item;
     boost::exception_ptr ex = error_;
-    
+
     if (items_.size() != 1)
     {
         ex = boost::copy_exception(RemoteCommsException("Unexpectedly received " + to_string(items_.size()) + " items from PROPFIND request"));

=== modified file 'src/RetrieveMetadataHandler.h'
--- src/RetrieveMetadataHandler.h	2016-11-01 02:53:02 +0000
+++ src/RetrieveMetadataHandler.h	2016-11-18 02:39:30 +0000
@@ -13,14 +13,13 @@
     typedef std::function<void(unity::storage::provider::Item const& item,
                                boost::exception_ptr const& error)> Callback;
 
-    RetrieveMetadataHandler(DavProvider const& provider,
+    RetrieveMetadataHandler(std::shared_ptr<DavProvider> const& provider,
                             std::string const& item_id,
                             unity::storage::provider::Context const& ctx,
                             Callback callback);
     ~RetrieveMetadataHandler();
 
 private:
-    std::string const item_id_;
     Callback const callback_;
 
 protected:

=== modified file 'src/RootsHandler.cpp'
--- src/RootsHandler.cpp	2016-10-31 01:40:28 +0000
+++ src/RootsHandler.cpp	2016-11-18 02:39:30 +0000
@@ -5,7 +5,8 @@
 using namespace std;
 using namespace unity::storage::provider;
 
-RootsHandler::RootsHandler(DavProvider const& provider, Context const& ctx)
+RootsHandler::RootsHandler(shared_ptr<DavProvider> const& provider,
+                           Context const& ctx)
     : PropFindHandler(provider, ".", 0, ctx)
 {
 }

=== modified file 'src/RootsHandler.h'
--- src/RootsHandler.h	2016-10-31 07:32:53 +0000
+++ src/RootsHandler.h	2016-11-18 02:39:30 +0000
@@ -9,7 +9,7 @@
 class RootsHandler : public PropFindHandler {
     Q_OBJECT
 public:
-    RootsHandler(DavProvider const& provider,
+    RootsHandler(std::shared_ptr<DavProvider> const& provider,
                  unity::storage::provider::Context const& ctx);
     ~RootsHandler();
 

=== added file 'src/http_error.cpp'
--- src/http_error.cpp	1970-01-01 00:00:00 +0000
+++ src/http_error.cpp	2016-11-18 02:39:30 +0000
@@ -0,0 +1,163 @@
+#include "http_error.h"
+
+#include <QNetworkReply>
+#include <QXmlDefaultHandler>
+#include <QXmlInputSource>
+#include <QXmlSimpleReader>
+#include <unity/storage/provider/Exceptions.h>
+
+#include <cassert>
+
+using namespace std;
+using namespace unity::storage::provider;
+
+namespace {
+
+class ErrorParser : public QXmlDefaultHandler
+{
+public:
+    bool startElement(QString const& namespace_uri, QString const& local_name,
+                      QString const& qname, QXmlAttributes const& attrs) override;
+    bool endElement(QString const& namespace_uri, QString const& local_name,
+                    QString const& qname) override;
+    bool characters(QString const& data) override;
+
+    string exception_;
+    string message_;
+
+private:
+    static QString const SABREDAV_NS;
+
+    QString char_data_;
+};
+
+QString const ErrorParser::SABREDAV_NS("http://sabredav.org/ns");
+
+bool ErrorParser::startElement(QString const& namespace_uri,
+                               QString const& local_name,
+                               QString const& qname,
+                               QXmlAttributes const& attrs)
+{
+    Q_UNUSED(namespace_uri);
+    Q_UNUSED(local_name);
+    Q_UNUSED(qname);
+    Q_UNUSED(attrs);
+
+    char_data_.clear();
+    return true;
+}
+
+bool ErrorParser::endElement(QString const& namespace_uri,
+                             QString const& local_name,
+                             QString const& qname)
+{
+    Q_UNUSED(qname);
+    if (namespace_uri == SABREDAV_NS)
+    {
+        if (local_name == "exception")
+        {
+            exception_ = char_data_.toStdString();
+        }
+        else if (local_name == "message")
+        {
+            message_ = char_data_.toStdString();
+        }
+    }
+    char_data_.clear();
+    return true;
+}
+
+bool ErrorParser::characters(QString const& data)
+{
+    char_data_ += data;
+    return true;
+}
+
+std::string make_message(QNetworkReply *reply,
+                         QByteArray body)
+{
+    if (body.isEmpty())
+    {
+        body = reply->readAll();
+    }
+    auto content_type = reply->header(QNetworkRequest::ContentTypeHeader).toString();
+    int semicolon = content_type.indexOf(';');
+    if (semicolon >= 0)
+    {
+        content_type = content_type.left(semicolon);
+    }
+    if (content_type == "text/plain")
+    {
+        return body.toStdString();
+    }
+    else if (content_type == "application/xml")
+    {
+        ErrorParser parser;
+        QXmlSimpleReader reader;
+        QXmlInputSource source;
+        reader.setContentHandler(&parser);
+        source.setData(body);
+        if (reader.parse(source)) {
+            return parser.exception_ + ": " + parser.message_;
+        }
+    }
+    // Fall back to returning the HTTP status string
+    return reply->attribute(
+        QNetworkRequest::HttpReasonPhraseAttribute).toString().toStdString();
+}
+
+}
+
+boost::exception_ptr translate_http_error(QNetworkReply *reply,
+                                          QByteArray const& body,
+                                          string const& item_id)
+{
+    assert(reply != nullptr);
+    assert(reply->isFinished());
+
+    auto status = reply->attribute(
+        QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    if (status == 0)
+    {
+        return boost::copy_exception(
+            RemoteCommsException(reply->errorString().toStdString()));
+    }
+
+    auto message = make_message(reply, body);
+
+    switch (status)
+    {
+    case 400: // Bad Request
+        return boost::copy_exception(RemoteCommsException(message));
+
+    case 401: // Unauthorised
+        // This should be separate from Forbidden, triggering reauth.
+    case 403: // Forbidden
+    case 451: // Unavailable for Legal Reasons
+        return boost::copy_exception(PermissionException(message));
+
+    case 404: // Not found
+    case 410: // Gone
+        return boost::copy_exception(NotExistsException(message, item_id));
+
+    case 405: // Method Not Allowed
+        // MKCOL on an existing resource results in a 405 error
+        if (reply->request().attribute(QNetworkRequest::CustomVerbAttribute) == "MKCOL")
+        {
+            return boost::copy_exception(ExistsException(message, item_id, "fixme"));
+        }
+        break;
+
+    case 409: // Conflict
+    case 412: // Precondition failed
+        return boost::copy_exception(ConflictException(message));
+
+    case 507: // Insufficient Storage
+        return boost::copy_exception(QuotaException(message));
+
+    default:
+        break;
+    }
+    return boost::copy_exception(
+        UnknownException("HTTP " + to_string(status) + ": " + message));
+}

=== added file 'src/http_error.h'
--- src/http_error.h	1970-01-01 00:00:00 +0000
+++ src/http_error.h	2016-11-18 02:39:30 +0000
@@ -0,0 +1,12 @@
+#pragma once
+
+#include <boost/thread.hpp>
+
+class QByteArray;
+class QNetworkReply;
+
+static constexpr int MAX_ERROR_BODY_LENGTH = 64 * 1024;
+
+boost::exception_ptr translate_http_error(QNetworkReply *reply,
+                                          QByteArray const& body,
+                                          std::string const& item_id={});

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-09-23 03:28:37 +0000
+++ tests/CMakeLists.txt	2016-11-18 02:39:30 +0000
@@ -15,6 +15,7 @@
   multistatus
   item_id
   davprovider
+  http_error
 )
 
 set(UNIT_TEST_TARGETS "")

=== modified file 'tests/davprovider/davprovider_test.cpp'
--- tests/davprovider/davprovider_test.cpp	2016-11-12 08:19:57 +0000
+++ tests/davprovider/davprovider_test.cpp	2016-11-18 02:39:30 +0000
@@ -95,7 +95,7 @@
 
         dav_env_.reset(new DavEnvironment(tmp_dir_->path()));
         provider_env_.reset(new ProviderEnvironment(
-                                unique_ptr<provider::ProviderBase>(new TestDavProvider(dav_env_->base_url())),
+                                make_shared<TestDavProvider>(dav_env_->base_url()),
                                 1, *dbus_env_));
     }
 
@@ -171,8 +171,7 @@
     auto account = get_client();
     make_file("foo.txt");
     make_file("bar.txt");
-    //make_dir("folder");
-    make_file("folder");
+    make_dir("folder");
     make_file("I\u00F1t\u00EBrn\u00E2ti\u00F4n\u00E0liz\u00E6ti\u00F8n");
 
     shared_ptr<Root> root;
@@ -217,16 +216,10 @@
     EXPECT_EQ("bar.txt", items[1]->name());
     EXPECT_EQ(ItemType::file, items[1]->type());
 
-    // FIXME: SabreDAV doesn't provide an ETag for folders, which
-    // currently trips up storage-framework's client side metadata
-    // validation.
-
-    //EXPECT_EQ("folder/", items[2]->native_identity());
-    EXPECT_EQ("folder", items[2]->native_identity());
+    EXPECT_EQ("folder/", items[2]->native_identity());
     EXPECT_EQ(".", items[2]->parent_ids().at(0));
     EXPECT_EQ("folder", items[2]->name());
-    //EXPECT_EQ(ItemType::folder, items[2]->type());
-    EXPECT_EQ(ItemType::file, items[2]->type());
+    EXPECT_EQ(ItemType::folder, items[2]->type());
 
     EXPECT_EQ("foo.txt", items[3]->native_identity());
     EXPECT_EQ(".", items[3]->parent_ids().at(0));
@@ -309,6 +302,46 @@
     EXPECT_EQ(ItemType::file, item->type());
 }
 
+TEST_F(DavProviderTests, metadata_not_found)
+{
+    auto account = get_client();
+
+    shared_ptr<Root> root;
+    {
+        QFutureWatcher<QVector<shared_ptr<Root>>> watcher;
+        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
+        watcher.setFuture(account->roots());
+        if (spy.count() == 0)
+        {
+            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
+        }
+        auto roots = watcher.result();
+        ASSERT_EQ(1, roots.size());
+        root = roots[0];
+    }
+
+    shared_ptr<Item> item;
+    {
+        QFutureWatcher<shared_ptr<Item>> watcher;
+        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
+        watcher.setFuture(root->get("foo.txt"));
+        if (spy.count() == 0)
+        {
+            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
+        }
+        try
+        {
+            watcher.result();
+            FAIL();
+        }
+        catch (NotExistsException const& e)
+        {
+            EXPECT_TRUE(e.error_message().startsWith("Sabre\\DAV\\Exception\\NotFound: "))
+                << e.error_message().toStdString();
+        }
+    }
+}
+
 TEST_F(DavProviderTests, create_folder)
 {
     auto account = get_client();
@@ -327,11 +360,6 @@
         root = roots[0];
     }
 
-    // FIXME: the test webdav server returns no ETag for folders, so
-    // the client throws away the create_folder() response.  We can
-    // reenable this when porting to storage-framework 0.2.
-    return;
-
     shared_ptr<Folder> folder;
     {
         QFutureWatcher<shared_ptr<Folder>> watcher;
@@ -382,9 +410,9 @@
             watcher.result();
             FAIL();
         }
-        catch (RemoteCommsException const& e)
+        catch (ExistsException const& e)
         {
-            EXPECT_EQ("Error from MKCOL: 405", e.error_message());
+            EXPECT_EQ("Sabre\\DAV\\Exception\\MethodNotAllowed: The resource you tried to create already exists", e.error_message());
         }
     }
 }
@@ -421,9 +449,9 @@
             watcher.result();
             FAIL();
         }
-        catch (RemoteCommsException const& e)
+        catch (ExistsException const& e)
         {
-            EXPECT_EQ("Error from MKCOL: 405", e.error_message());
+            EXPECT_EQ("Sabre\\DAV\\Exception\\MethodNotAllowed: The resource you tried to create already exists", e.error_message());
         }
     }
 }
@@ -537,9 +565,9 @@
             watcher.result();
             FAIL();
         }
-        catch (RemoteCommsException const& e)
+        catch (ConflictException const& e)
         {
-            EXPECT_EQ("Error from QNetworkReply: 299", e.error_message());
+            EXPECT_EQ("Sabre\\DAV\\Exception\\PreconditionFailed: An If-None-Match header was specified, but the ETag matched (or * was specified).", e.error_message());
         }
     }
 }
@@ -686,9 +714,9 @@
             watcher.result();
             FAIL();
         }
-        catch (RemoteCommsException const& e)
+        catch (ConflictException const& e)
         {
-            EXPECT_EQ("Error from QNetworkReply: 299", e.error_message());
+            EXPECT_EQ("Sabre\\DAV\\Exception\\PreconditionFailed: An If-Match header was specified, but none of the specified the ETags matched.", e.error_message());
         }
     }
 }
@@ -738,7 +766,7 @@
         }
         catch (RemoteCommsException const& e)
         {
-            EXPECT_EQ("Error from QNetworkReply: 99", e.error_message());
+            EXPECT_EQ("Unknown error", e.error_message());
         }
     }
 }
@@ -1017,9 +1045,10 @@
             watcher.waitForFinished(); // to check for errors
             FAIL();
         }
-        catch (RemoteCommsException const& e)
+        catch (NotExistsException const& e)
         {
-            EXPECT_EQ("Unexpected status code: 404", e.error_message());
+            EXPECT_TRUE(e.error_message().startsWith("Sabre\\DAV\\Exception\\NotFound: "))
+                << e.error_message().toStdString();
         }
     }
 }
@@ -1117,9 +1146,11 @@
             watcher.waitForFinished(); // to catch any errors
             FAIL();
         }
-        catch (RemoteCommsException const& e)
+        catch (NotExistsException const& e)
         {
-            EXPECT_EQ("Error from DELETE: 404", e.error_message());
+            EXPECT_TRUE(e.error_message().startsWith(
+                            "Sabre\\DAV\\Exception\\NotFound: "))
+                << e.error_message().toStdString();
         }
     }
 }

=== added directory 'tests/http_error'
=== added file 'tests/http_error/CMakeLists.txt'
--- tests/http_error/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/http_error/CMakeLists.txt	2016-11-18 02:39:30 +0000
@@ -0,0 +1,8 @@
+add_executable(http_error_test http_error_test.cpp)
+target_link_libraries(http_error_test
+  dav-provider-lib
+  testutils
+  Qt5::Test
+  gtest
+)
+add_test(http_error_test http_error_test)

=== added file 'tests/http_error/http_error_test.cpp'
--- tests/http_error/http_error_test.cpp	1970-01-01 00:00:00 +0000
+++ tests/http_error/http_error_test.cpp	2016-11-18 02:39:30 +0000
@@ -0,0 +1,260 @@
+#include "../../src/http_error.h"
+
+#include <gtest/gtest.h>
+#include <QBuffer>
+#include <QByteArray>
+#include <QCoreApplication>
+#include <QNetworkReply>
+#include <QNetworkRequest>
+#include <unity/storage/provider/Exceptions.h>
+
+using namespace std;
+using namespace unity::storage::provider;
+
+class FakeReply : public QNetworkReply
+{
+public:
+    FakeReply(QString const& method, int status, QString const& reason,
+              QString const& content_type, QByteArray const& body)
+    {
+        QNetworkRequest request;
+        request.setAttribute(QNetworkRequest::CustomVerbAttribute, method);
+        setRequest(request);
+
+        setAttribute(QNetworkRequest::HttpStatusCodeAttribute, status);
+        setAttribute(QNetworkRequest::HttpReasonPhraseAttribute, reason);
+
+        setHeader(QNetworkRequest::ContentTypeHeader, content_type);
+        setHeader(QNetworkRequest::ContentLengthHeader, body.size());
+        buffer_.setData(body);
+        buffer_.open(QIODevice::ReadOnly);
+        open(QIODevice::ReadOnly);
+        setFinished(true);
+    }
+
+    FakeReply(QNetworkReply::NetworkError code, QString const& message)
+    {
+        setError(code, message);
+        setFinished(true);
+    }
+
+    void abort() override
+    {
+        close();
+    }
+
+    qint64 bytesAvailable() const override
+    {
+        return QNetworkReply::bytesAvailable() + buffer_.bytesAvailable();
+    }
+
+    bool isSequential() const override
+    {
+        return true;
+    }
+
+    qint64 size() const override
+    {
+        return buffer_.size();
+    }
+
+    qint64 readData(char *data, qint64 length) override
+    {
+        return buffer_.read(data, length);
+    }
+
+private:
+    QBuffer buffer_;
+};
+
+template <typename E>
+E expect_throw(boost::exception_ptr p)
+{
+    try
+    {
+        boost::rethrow_exception(p);
+    }
+    catch (E const& e)
+    {
+        return e;
+    }
+    throw std::runtime_error("No exception thrown");
+}
+
+
+TEST(HttpErrorTests, network_error)
+{
+    FakeReply reply(QNetworkReply::HostNotFoundError, "Host not found");
+
+    auto e = expect_throw<RemoteCommsException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("Host not found", e.error_message());
+}
+
+TEST(HttpErrorTests, text_plain_body)
+{
+    FakeReply reply("GET", 400, "Bad Request",
+                    "text/plain;charset=US-ASCII", "error message");
+    auto e = expect_throw<RemoteCommsException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("error message", e.error_message());
+}
+
+TEST(HttpErrorTests, application_xml_body)
+{
+    static const char xml[] = R"(<?xml version="1.0" encoding="utf-8"?>
+<d:error xmlns:d='DAV:' xmlns:s='http://sabredav.org/ns'>
+  <s:exception>exception_name</s:exception>
+  <s:message>error message</s:message>
+  <s:sabredav-version>1.8.12</s:sabredav-version>
+</d:error>
+)";
+    FakeReply reply("GET", 400, "Bad Request",
+                    "application/xml;charset=utf-8", xml);
+
+    auto e = expect_throw<RemoteCommsException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("exception_name: error message", e.error_message());
+}
+
+TEST(HttpErrorTests, fallback_body)
+{
+    FakeReply reply("GET", 400, "Bad Request",
+                    "application/octet-stream", "foo");
+    auto e = expect_throw<RemoteCommsException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("Bad Request", e.error_message());
+}
+
+TEST(HttpErrorTests, body_provided_separately)
+{
+    static const char xml[] = R"(<?xml version="1.0" encoding="utf-8"?>
+<d:error xmlns:d='DAV:' xmlns:s='http://sabredav.org/ns'>
+  <s:exception>exception_name</s:exception>
+  <s:message>error message</s:message>
+  <s:sabredav-version>1.8.12</s:sabredav-version>
+</d:error>
+)";
+    FakeReply reply("GET", 400, "Bad Request",
+                    "application/xml;charset=utf-8", "foo");
+
+    auto e = expect_throw<RemoteCommsException>(
+        translate_http_error(&reply, xml));
+    EXPECT_EQ("exception_name: error message", e.error_message());
+}
+
+TEST(HttpErrorTests, 401)
+{
+    FakeReply reply("GET", 401, "Unauthorised", "text/plain", "message");
+
+    auto e = expect_throw<PermissionException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("message", e.error_message());
+}
+
+TEST(HttpErrorTests, 403)
+{
+    FakeReply reply("GET", 403, "Forbidden", "text/plain", "message");
+
+    auto e = expect_throw<PermissionException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("message", e.error_message());
+}
+
+TEST(HttpErrorTests, 451)
+{
+    FakeReply reply("GET", 451, "Unavailable for Legal Reasons",
+                    "text/plain", "message");
+
+    auto e = expect_throw<PermissionException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("message", e.error_message());
+}
+
+TEST(HttpErrorTests, 404)
+{
+    FakeReply reply("GET", 404, "Not Found", "text/plain", "File not found");
+
+    auto e = expect_throw<NotExistsException>(
+        translate_http_error(&reply, QByteArray(), "item_id"));
+    EXPECT_EQ("File not found", e.error_message());
+    EXPECT_EQ("item_id", e.key());
+}
+
+TEST(HttpErrorTests, 410)
+{
+    FakeReply reply("GET", 410, "Gone", "text/plain", "File not found");
+
+    auto e = expect_throw<NotExistsException>(
+        translate_http_error(&reply, QByteArray(), "item_id"));
+    EXPECT_EQ("File not found", e.error_message());
+    EXPECT_EQ("item_id", e.key());
+}
+
+TEST(HttpErrorTests, 405)
+{
+    // 405 for MKCOL means the target exists
+    {
+        FakeReply reply("MKCOL", 405, "Method Not Allowed",
+                        "text/plain", "message");
+
+        auto e = expect_throw<ExistsException>(
+            translate_http_error(&reply, QByteArray(), "item_id"));
+        EXPECT_EQ("message", e.error_message());
+        EXPECT_EQ("item_id", e.native_identity());
+    }
+    // Otherwise, it is unknown
+    {
+        FakeReply reply("GET", 405, "Method Not Allowed",
+                        "text/plain", "message");
+
+        auto e = expect_throw<UnknownException>(
+            translate_http_error(&reply, QByteArray(), "item_id"));
+        EXPECT_EQ("HTTP 405: message", e.error_message());
+    }
+}
+
+TEST(HttpErrorTests, 409)
+{
+    FakeReply reply("GET", 409, "Conflict", "text/plain", "message");
+
+    auto e = expect_throw<ConflictException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("message", e.error_message());
+}
+
+TEST(HttpErrorTests, 412)
+{
+    FakeReply reply("GET", 412, "Precondition failed", "text/plain", "message");
+
+    auto e = expect_throw<ConflictException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("message", e.error_message());
+}
+
+TEST(HttpErrorTests, 507)
+{
+    FakeReply reply("GET", 507, "Insufficient Storage",
+                    "text/plain", "message");
+
+    auto e = expect_throw<QuotaException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("message", e.error_message());
+}
+
+TEST(HttpErrorTests, other)
+{
+    FakeReply reply("GET", 500, "Internal Server Error",
+                    "text/plain", "message");
+
+    auto e = expect_throw<UnknownException>(
+        translate_http_error(&reply, QByteArray()));
+    EXPECT_EQ("HTTP 500: message", e.error_message());
+}
+
+int main(int argc, char**argv)
+{
+    QCoreApplication app(argc, argv);
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}

=== modified file 'tests/utils/ProviderEnvironment.cpp'
--- tests/utils/ProviderEnvironment.cpp	2016-09-28 15:26:03 +0000
+++ tests/utils/ProviderEnvironment.cpp	2016-11-18 02:39:30 +0000
@@ -16,7 +16,7 @@
 
 }
 
-ProviderEnvironment::ProviderEnvironment(unique_ptr<ProviderBase>&& provider,
+ProviderEnvironment::ProviderEnvironment(shared_ptr<ProviderBase> const& provider,
                                          OnlineAccounts::AccountId account_id,
                                          DBusEnvironment const& dbus_env)
 {
@@ -29,7 +29,7 @@
     account_manager_->waitForReady();
     OnlineAccounts::Account* account = account_manager_->account(account_id);
     assert(account != nullptr);
-    server_.reset(new testing::TestServer(move(provider), account,
+    server_.reset(new testing::TestServer(provider, account,
                                           *server_connection_,
                                           OBJECT_PATH.toStdString()));
 

=== modified file 'tests/utils/ProviderEnvironment.h'
--- tests/utils/ProviderEnvironment.h	2016-09-19 11:01:16 +0000
+++ tests/utils/ProviderEnvironment.h	2016-11-18 02:39:30 +0000
@@ -12,7 +12,7 @@
 class ProviderEnvironment
 {
 public:
-    ProviderEnvironment(std::unique_ptr<unity::storage::provider::ProviderBase>&& provider,
+    ProviderEnvironment(std::shared_ptr<unity::storage::provider::ProviderBase> const& provider,
                         OnlineAccounts::AccountId account_id,
                         DBusEnvironment const& dbus_env);
     ~ProviderEnvironment();