Ayende @ Rahien

It's a girl

RavenDB implementation frustrations

While working on the RavenDB server is usually a lot of fun, there is a part of RavenDB client that I absolutely abhor. Every time that I need to touch the part of the client API that talks to the server, it is a pain. Why is that?

Let us take a the simple example, loading a document by id. On the wire, it looks like this:

GET /docs/users/ayende

It can’t be simpler than that, except that internally in RavenDB, we have three implementation for that:

  • Standard sync impl
  • Standard async impl
  • Silverlight async impl

The problem is that each of those uses different API, and while we created a shared abstraction for async / sync, at least, it is a hard task making sure that they are all in sync with one another if we need to make a modification.

For example, let us look at the three implementation of Get:

public JsonDocument DirectGet(string serverUrl, string key)
{
    var metadata = new RavenJObject();
    AddTransactionInformation(metadata);
    var request = jsonRequestFactory.CreateHttpJsonRequest(this, serverUrl + "/docs/" + key, "GET", metadata, credentials, convention);
    request.AddOperationHeaders(OperationsHeaders);
    try
    {
        var requestString = request.ReadResponseString();
        RavenJObject meta = null;
        RavenJObject jsonData = null;
        try
        {
            jsonData = RavenJObject.Parse(requestString);
            meta = request.ResponseHeaders.FilterHeaders(isServerDocument: false);
        }
        catch (JsonReaderException jre)
        {
            var headers = "";
            foreach (string header in request.ResponseHeaders)
            {
                headers = headers + string.Format("\n\r{0}:{1}", header, request.ResponseHeaders[header]);
            }
            throw new JsonReaderException("Invalid Json Response: \n\rHeaders:\n\r" + headers + "\n\rBody:" + requestString, jre);
        }
        return new JsonDocument
        {
            DataAsJson = jsonData,
            NonAuthoritiveInformation = request.ResponseStatusCode == HttpStatusCode.NonAuthoritativeInformation,
            Key = key,
            Etag = new Guid(request.ResponseHeaders["ETag"]),
            LastModified = DateTime.ParseExact(request.ResponseHeaders["Last-Modified"], "r", CultureInfo.InvariantCulture).ToLocalTime(),
            Metadata = meta
        };
    }
    catch (WebException e)
    {
        var httpWebResponse = e.Response as HttpWebResponse;
        if (httpWebResponse == null)
            throw;
        if (httpWebResponse.StatusCode == HttpStatusCode.NotFound)
            return null;
        if (httpWebResponse.StatusCode == HttpStatusCode.Conflict)
        {
            var conflicts = new StreamReader(httpWebResponse.GetResponseStreamWithHttpDecompression());
            var conflictsDoc = RavenJObject.Load(new JsonTextReader(conflicts));
            var conflictIds = conflictsDoc.Value<RavenJArray>("Conflicts").Select(x => x.Value<string>()).ToArray();

            throw new ConflictException("Conflict detected on " + key +
                                        ", conflict must be resolved before the document will be accessible")
            {
                ConflictedVersionIds = conflictIds
            };
        }
        throw;
    }
}
 

This is the sync API, of course, next we will look at the same method, for the full .NET framework TPL:

public Task<JsonDocument> GetAsync(string key)
{
    EnsureIsNotNullOrEmpty(key, "key");

    var metadata = new RavenJObject();
    AddTransactionInformation(metadata);
    var request = jsonRequestFactory.CreateHttpJsonRequest(this, url + "/docs/" + key, "GET", metadata, credentials, convention);

    return Task.Factory.FromAsync<string>(request.BeginReadResponseString, request.EndReadResponseString, null)
        .ContinueWith(task =>
        {
            try
            {
                var responseString = task.Result;
                return new JsonDocument
                {
                    DataAsJson = RavenJObject.Parse(responseString),
                    NonAuthoritiveInformation = request.ResponseStatusCode == HttpStatusCode.NonAuthoritativeInformation,
                    Key = key,
                    LastModified = DateTime.ParseExact(request.ResponseHeaders["Last-Modified"], "r", CultureInfo.InvariantCulture).ToLocalTime(),
                    Etag = new Guid(request.ResponseHeaders["ETag"]),
                    Metadata = request.ResponseHeaders.FilterHeaders(isServerDocument: false)
                };
            }
            catch (WebException e)
            {
                var httpWebResponse = e.Response as HttpWebResponse;
                if (httpWebResponse == null)
                    throw;
                if (httpWebResponse.StatusCode == HttpStatusCode.NotFound)
                    return null;
                if (httpWebResponse.StatusCode == HttpStatusCode.Conflict)
                {
                    var conflicts = new StreamReader(httpWebResponse.GetResponseStreamWithHttpDecompression());
                    var conflictsDoc = RavenJObject.Load(new JsonTextReader(conflicts));
                    var conflictIds = conflictsDoc.Value<RavenJArray>("Conflicts").Select(x => x.Value<string>()).ToArray();

                    throw new ConflictException("Conflict detected on " + key +
                                                ", conflict must be resolved before the document will be accessible")
                    {
                        ConflictedVersionIds = conflictIds
                    };
                }
                throw;
            }
        });
}

And here is the Siliverlight version:

public Task<JsonDocument> GetAsync(string key)
{
    EnsureIsNotNullOrEmpty(key, "key");

    key = key.Replace("\\",@"/"); //NOTE: the present of \ causes the SL networking stack to barf, even though the Uri seemingly makes this translation itself

    var request = url.Docs(key)
        .ToJsonRequest(this, credentials, convention);

    return request
        .ReadResponseStringAsync()
        .ContinueWith(task =>
        {
            try
            {
                var responseString = task.Result;
                return new JsonDocument
                {
                    DataAsJson = RavenJObject.Parse(responseString),
                    NonAuthoritiveInformation = request.ResponseStatusCode == HttpStatusCode.NonAuthoritativeInformation,
                    Key = key,
                    LastModified = DateTime.ParseExact(request.ResponseHeaders["Last-Modified"].First(), "r", CultureInfo.InvariantCulture).ToLocalTime(),
                    Etag = new Guid(request.ResponseHeaders["ETag"].First()),
                    Metadata = request.ResponseHeaders.FilterHeaders(isServerDocument: false)
                };
            }
            catch (AggregateException e)
            {
                var webException = e.ExtractSingleInnerException() as WebException;
                if (webException != null)
                {
                    if (HandleWebExceptionForGetAsync(key, webException))
                        return null;
                }
                throw;
            }
            catch (WebException e)
            {
                if (HandleWebExceptionForGetAsync(key, e))
                    return null;
                throw;
            }
        });
}

Did I mention it is annoying?

All of those methods are doing the exact same thing, but I have to maintain 3 versions of them. I thought about dropping the sync version (it is easy to do sync on top of async), which would mean that I would have only 2 implementations, but I don’t like it. The error handling and debugging support for async is still way below what you can get for sync code.

I don’t know if there is a solution for this, I just know that this is a big pain point for me.

Tags:

Posted By: Ayende Rahien

Published at

Originally posted at

Comments

Simon Bartlett
08/09/2011 09:26 AM by
Simon Bartlett

You should make your code samples scrollable - they disappear off the right side of the page behind the main navigation.

tarwn
08/09/2011 10:52 AM by
tarwn

Any reason not to extract the central portion of the sync method (the outer try/catch) and use it as a method call for the sync and asyc operation? Other than the extra try/catch I'm not seeing much difference between those two (I'm also on my first cup of coffee). If you refactored that into a method then you could easily use it from both the sync and async operation. The silverlight one looks fairly different due to the error catching, but it looks like you have already started extracting the web exception handling into a different function? It seems there might be some potential to use a common webexception handle function. Anyways, as I said earlier I'm on my first cup of coffee and it's possible that something like this has already occurred to you and isn't possible (sometimes things look possible w/ code samples that aren't when you take the whole codebase into account).

tobi
08/09/2011 11:33 AM by
tobi

One thing that occured to me very late is that very very similar algorithms sometimes have special cases in only a few places but you just cannot either pull out the similarities or generalize using parameters/delegates. Sometimes, it is just not possible.

Therefore I try to keep the different versions in sync as much as possible and keep as much of the structure the same as I can. Basically I just give up. Once you realize that this case actually occurs (and it is not just you being stupid) you take it much more easily.

Andres
08/09/2011 01:51 PM by
Andres

The construction of the JsonDocument should be in a method, there are a lot of common code inside these brackets. The URL generation could be in another method too, and maybe some of the exception management

Uncle Lob
08/10/2011 08:10 AM by
Uncle Lob

Wow that is one ugly code. Have you read Clean Code book sir?

Andrey Shchekin
08/10/2011 10:01 PM by
Andrey Shchekin

I might be completely wrong in my understanding of the underlying problem, but I would have solved this thing this way: public IValue Get(string serverUrl, string key, Func<Request, IValue> getResponseString)

where interface IValue is a monad-like thing: public interface IValue { IValue Apply(Func<T, TResult> function); ... }

Then you have two implementations of IValue, Just where Apply runs immediately and Furture, where Apply uses ContinueWith.

Ayende Rahien
08/10/2011 11:19 PM by
Ayende Rahien

Andrey, Try to write an API like that, especially an Async one. It isn't pleasant. And it doesn't really work when you have exceptional cases or need to do retries

Patrick Huizinga
08/11/2011 08:15 AM by
Patrick Huizinga

What prevents you from reusing the Silverlight code in the full .Net stack? Besides the few lines for like transactions that could be #if-ed?

However, I feel your pain. I implemented a cross platform duplex channel (that actually stays open), does retries and reuses its underlying channels. It took me about a thousand lines (including comments and white lines) and I had to get my hands dirty with IRequestChannel and custom Messages. Oh and I the server side was another few hundred lines (partly shared with the client side).

Btw, in the Silverlight code I see a ContinueWith call. Is that from an actual Task, or is that custom code that just resembles a Task?

Ayende Rahien
08/11/2011 10:45 AM by
Ayende Rahien

Patrick, The networking stack difference means that I have slight different lower level stuff in the SL codebase, requiring me to create an abstraction on top of that, but that abstraction is useful when we are at that level, I have to have a different way to process for async in SL and in standard code. The main problem with the ifdef approach isn't for that, the problem is that three different ways of invoking the same logic (sync, async, lazy).

Comments have been closed on this topic.