Azure-cosmos-dotnet-v2: Stop 404s from causing a DocumentClientException

[kieronlanning]

Make use of the status code that is already part of the API.

For example, await _client.ReadDocumentAsync(documentIdUri, new RequestOptions()) currently throws a DocumentClientException, instead use the StatusCode property to check.

Currently, we have to do this:

            {
                var response = await _client.ReadDocumentAsync(documentIdUri, new RequestOptions()).ConfigureAwait(false);

                string content;
                using (var reader = new StreamReader(response.ResponseStream, Encoding.UTF8, true, 2048))
                    content = await reader.ReadToEndAsync().ConfigureAwait(false);

                return JsonConvert.DeserializeObject<T>(content, JsonSerializerSettings);
            }
            catch(DocumentClientException exception)
            {
                if (exception.StatusCode == System.Net.HttpStatusCode.NotFound)
                    return default(T);

                throw;
            }

Instead, we should be able to do this:

                var response = await _client.ReadDocumentAsync(documentIdUri, new RequestOptions()).ConfigureAwait(false);
                if (response.StatusCode == System.Net.HttpStatusCode.NotFound)
                    return default(T);

                string content;
                using (var reader = new StreamReader(response.ResponseStream, Encoding.UTF8, true, 2048))
                    content = await reader.ReadToEndAsync().ConfigureAwait(false);

                return JsonConvert.DeserializeObject<T>(content, JsonSerializerSettings);

This could be a client setting, or a RequestOptions setting.

[rnagpal]

@kieronlanning Any reason why you closed this? I'll reopen and add it to our backlog.

[kieronlanning]

@rnagpal I've no idea! It wasn't on purpose...! Sorry about that, thanks for re-opening.

[jlchmura]

+1

[rikard-engstrom]

+1

[northtyphoon]

+1 I also noticed ~1000ms performance penalty associated with the exception.

[EricDahlvang]

+1

[reddy6ue]

+1

[joshlang]

+1

A 404 result is not exceptional.

In our use case, we check for the existence of thousands of items. Only a couple matches are expected. The other thousands of requests would return 404, as expected.

An exception seems to take about 0.2ms in production.
A read from cosmos usually takes 1-2ms.

Therefore, there's a 10-20% latency penalty for "no good reason". NOT reading something should be FASTER than reading something, not slower.

I could do a query by id, but that's discouraged, and costs 2.5x as much, according to this: https://docs.microsoft.com/en-us/azure/cosmos-db/request-units

[themajix]

Any updates on this issue? It has been more than 2 years since it was reported.

[joshlang]

There's a new SDK version coming out which addresses it for the SQL API, and maybe the table one - not sure. Check it out - it's in preview

[j82w]

This is fixed in the v3 SDK which is open sourced. It's currently in preview, but that will change soon.

[serhatozgel]

Hi @j82w, I have Microsoft.Azure.Cosmos 3.5.1 installed and this method still throws an exception:

Container c = ...;
c.ReadItemAsync<T>(id, new PartitionKey(id));

Is this expected?

[j82w]

During the v3 preview there was a lot of feedback especially from people porting from v2 that not throwing on just 404 was unexpected and painful since it was only a runtime error. Based on the feedback it was decided to continue to throw on 404 for the typed APIs.

If you're looking for performance then you should use the stream APIs. They do not throw any of the service exceptions including 404.

[serhatozgel]

@j82w Right, migration would be painful. I would suggest adding a new method, such as Task<T?> GetItemAsync() that returns null and deprecate the ReadItemAsync over time.

[kieronlanning]

Couldn't this be an option in the ItemRequestOptions or even when creating the CosmosClientOptions?

[j82w]

Looking at the overall user experience making 404 return null instead of throwing adds a lot of complexity. The SDK currently throws all exceptions. Having the SDK throw some exceptions and not others is just confusing for everyone. The better design is to move to an entirely exceptionalness design which is what we did with the stream APIS. Another alternative is to write a custom wrapper implementation that handles the 404 and returns null.

@kieronlanning This issue was already discussed.

@serhatozgel we did add the new stream APIs which don't throw.

[kieronlanning]

I'm going to close this, I'm using the streaming APIs in the v3 client, and I'm happy with that for now.

Thanks for the ongoing dicussion.