Skip to content

Commit 29c6836

Browse files
committed
Address comments and update tests
1 parent 820c460 commit 29c6836

2 files changed

Lines changed: 118 additions & 26 deletions

File tree

Microsoft.Azure.Cosmos/src/Routing/ClientCollectionCache.cs

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -156,16 +156,35 @@ private async Task<ContainerProperties> ResolveCollectionWithSessionContainerCle
156156
{
157157
string previouslyResolvedCollectionRid = request?.RequestContext?.ResolvedCollectionRid;
158158

159-
ContainerProperties properties = await resolveContainerProvider();
160-
161-
if (this.sessionContainer != null &&
162-
previouslyResolvedCollectionRid != null &&
163-
previouslyResolvedCollectionRid != properties.ResourceId)
159+
try
164160
{
165-
this.sessionContainer.ClearTokenByResourceId(previouslyResolvedCollectionRid);
161+
ContainerProperties properties = await resolveContainerProvider();
162+
163+
if (this.sessionContainer != null &&
164+
previouslyResolvedCollectionRid != null &&
165+
previouslyResolvedCollectionRid != properties.ResourceId)
166+
{
167+
this.sessionContainer.ClearTokenByResourceId(previouslyResolvedCollectionRid);
168+
}
169+
170+
return properties;
166171
}
172+
catch (DocumentClientException ex)
173+
{
174+
// When collection resolution fails with 404, set the appropriate substatus code.
175+
// - For document/item operations: Set substatus 1003 (OwnerResourceNotFound) to distinguish
176+
// "container doesn't exist" from "item doesn't exist" (substatus 0).
177+
// - For container operations: Leave substatus as 0 because the container itself is the
178+
// resource that doesn't exist (not an "owner" resource issue).
179+
if (ex.StatusCode == System.Net.HttpStatusCode.NotFound &&
180+
ex.GetSubStatus() == SubStatusCodes.Unknown &&
181+
request?.ResourceType != ResourceType.Collection)
182+
{
183+
ex.Headers[WFConstants.BackendHeaders.SubStatus] = ((uint)SubStatusCodes.OwnerResourceNotFound).ToString(System.Globalization.CultureInfo.InvariantCulture);
184+
}
167185

168-
return properties;
186+
throw;
187+
}
169188
}
170189

171190
private async Task<ContainerProperties> ReadCollectionAsync(
@@ -237,19 +256,11 @@ await this.storeModel.ProcessMessageAsync(request))
237256
catch (DocumentClientException ex)
238257
{
239258
childTrace.AddDatum("Exception Message", ex.Message);
240-
241-
// When a collection read fails with 404, it means the owner resource (container/database) doesn't exist.
242-
// Ensure the substatus code is set to 1003 (OwnerResourceNotFound) to distinguish from item-not-found scenarios.
243-
if (ex.StatusCode == System.Net.HttpStatusCode.NotFound && ex.GetSubStatus() == SubStatusCodes.Unknown)
244-
{
245-
ex.Headers[WFConstants.BackendHeaders.SubStatus] = ((uint)SubStatusCodes.OwnerResourceNotFound).ToString(System.Globalization.CultureInfo.InvariantCulture);
246-
}
247-
248259
throw;
249260
}
250261
}
251262
}
252263
}
253264
}
254265
}
255-
}
266+
}

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosNotFoundTests.cs

Lines changed: 91 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,11 @@ public async Task ValidateQueryNotFoundResponse()
9999

100100
/// <summary>
101101
/// Validates that 404 with substatus 0 is returned when an item doesn't exist,
102-
/// and 404 with substatus 1003 is returned when the container doesn't exist.
102+
/// and 404 with substatus 1003 is returned when the container doesn't exist (Direct mode only).
103103
/// This allows disambiguation between item not found vs owner resource (container/database) not found.
104+
///
105+
/// Note: Gateway mode has a known limitation where it cannot always distinguish between
106+
/// item-not-found and container-not-found scenarios. Direct mode properly sets substatus 1003.
104107
/// </summary>
105108
[TestMethod]
106109
[DataRow(true, DisplayName = "Gateway mode")]
@@ -124,18 +127,33 @@ public async Task ValidateSubStatusCodeForItemNotFoundVsContainerNotFound(bool u
124127
Assert.AreEqual(0, (int)response.Headers.SubStatusCode,
125128
"SubStatusCode should be 0 when item doesn't exist in an existing container");
126129

127-
// Test 2: Container doesn't exist - should return 404 with substatus 1003
130+
// Test 2: Container doesn't exist
131+
// Direct mode: should return 404 with substatus 1003
132+
// Gateway mode: returns 404 with substatus 0 (known limitation)
128133
Container nonExistentContainer = db.GetContainer(DoesNotExist);
129134
response = await nonExistentContainer.ReadItemStreamAsync(
130135
partitionKey: new Cosmos.PartitionKey(DoesNotExist),
131136
id: DoesNotExist);
132137

133138
Assert.IsNotNull(response);
134139
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
135-
Assert.AreEqual(1003, (int)response.Headers.SubStatusCode,
136-
"SubStatusCode should be 1003 when container doesn't exist (owner resource not found)");
140+
141+
if (!useGateway)
142+
{
143+
// Direct mode can distinguish container-not-found from item-not-found
144+
Assert.AreEqual(1003, (int)response.Headers.SubStatusCode,
145+
"SubStatusCode should be 1003 when container doesn't exist (owner resource not found)");
146+
}
147+
else
148+
{
149+
// Gateway mode limitation: Cannot always distinguish, returns substatus 0
150+
Assert.AreEqual(0, (int)response.Headers.SubStatusCode,
151+
"Gateway mode returns substatus 0 (known limitation - cannot distinguish container-not-found from item-not-found)");
152+
}
137153

138-
// Test 3: Database doesn't exist - should also return 404 with substatus 1003
154+
// Test 3: Database doesn't exist
155+
// Both Direct and Gateway mode should return 404 with substatus 1003
156+
// because the collection cache detects the database doesn't exist
139157
Database nonExistentDb = testClient.GetDatabase(DoesNotExist);
140158
Container containerInNonExistentDb = nonExistentDb.GetContainer(DoesNotExist);
141159
response = await containerInNonExistentDb.ReadItemStreamAsync(
@@ -154,6 +172,9 @@ public async Task ValidateSubStatusCodeForItemNotFoundVsContainerNotFound(bool u
154172
/// <summary>
155173
/// Validates that CosmosException exposes the substatus code when thrown,
156174
/// allowing developers to distinguish between different types of 404 errors.
175+
///
176+
/// Note: Gateway mode has a known limitation where it cannot always distinguish between
177+
/// item-not-found and container-not-found scenarios. Direct mode properly sets substatus 1003.
157178
/// </summary>
158179
[TestMethod]
159180
[DataRow(true, DisplayName = "Gateway mode")]
@@ -181,7 +202,9 @@ public async Task ValidateCosmosExceptionSubStatusCodeForNotFound(bool useGatewa
181202
"SubStatusCode should be 0 when item doesn't exist in an existing container");
182203
}
183204

184-
// Test 2: Container doesn't exist - CosmosException should have substatus 1003
205+
// Test 2: Container doesn't exist
206+
// Direct mode: CosmosException should have substatus 1003
207+
// Gateway mode: CosmosException has substatus 0 (known limitation)
185208
Container nonExistentContainer = db.GetContainer(DoesNotExist);
186209
try
187210
{
@@ -192,11 +215,23 @@ public async Task ValidateCosmosExceptionSubStatusCodeForNotFound(bool useGatewa
192215
}
193216
catch (CosmosException ex) when (ex.StatusCode == HttpStatusCode.NotFound)
194217
{
195-
Assert.AreEqual(1003, ex.SubStatusCode,
196-
"SubStatusCode should be 1003 when container doesn't exist (owner resource not found)");
218+
if (!useGateway)
219+
{
220+
// Direct mode can distinguish container-not-found from item-not-found
221+
Assert.AreEqual(1003, ex.SubStatusCode,
222+
"SubStatusCode should be 1003 when container doesn't exist (owner resource not found)");
223+
}
224+
else
225+
{
226+
// Gateway mode limitation: Cannot always distinguish, returns substatus 0
227+
Assert.AreEqual(0, ex.SubStatusCode,
228+
"Gateway mode returns substatus 0 (known limitation - cannot distinguish container-not-found from item-not-found)");
229+
}
197230
}
198231

199-
// Test 3: Database doesn't exist - CosmosException should have substatus 1003
232+
// Test 3: Database doesn't exist
233+
// Both Direct and Gateway mode should return substatus 1003
234+
// because the collection cache detects the database doesn't exist
200235
Database nonExistentDb = testClient.GetDatabase(DoesNotExist);
201236
Container containerInNonExistentDb = nonExistentDb.GetContainer(DoesNotExist);
202237
try
@@ -281,6 +316,52 @@ private async Task ItemOperations(Container container, bool containerNotExist)
281316
streamPayload: replace));
282317
}
283318

319+
/// <summary>
320+
/// Validates that container metadata operations (ReadContainer, DeleteContainer) return 404 with substatus 0
321+
/// when the container doesn't exist, as opposed to item operations which return substatus 1003.
322+
/// This ensures we don't incorrectly set substatus 1003 for container-level operations.
323+
/// </summary>
324+
[TestMethod]
325+
[DataRow(true, DisplayName = "Gateway mode")]
326+
[DataRow(false, DisplayName = "Direct mode")]
327+
public async Task ValidateContainerMetadataRequestsHaveSubstatus0(bool useGateway)
328+
{
329+
// Create a test client with the specified mode
330+
using CosmosClient testClient = TestCommon.CreateCosmosClient(useGateway);
331+
332+
// Create a test database
333+
Database db = await testClient.CreateDatabaseAsync("NotFoundTest" + Guid.NewGuid().ToString());
334+
335+
// Test 1: ReadContainer on non-existent container should return 404 with substatus 0
336+
Container nonExistentContainer = db.GetContainer(DoesNotExist);
337+
ResponseMessage response = await nonExistentContainer.ReadContainerStreamAsync();
338+
339+
Assert.IsNotNull(response);
340+
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
341+
Assert.AreEqual(0, (int)response.Headers.SubStatusCode,
342+
"ReadContainer should return substatus 0 when container doesn't exist (the container itself is the missing resource)");
343+
344+
// Test 2: DeleteContainer on non-existent container should also return 404 with substatus 0
345+
response = await nonExistentContainer.DeleteContainerStreamAsync();
346+
347+
Assert.IsNotNull(response);
348+
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
349+
Assert.AreEqual(0, (int)response.Headers.SubStatusCode,
350+
"DeleteContainer should return substatus 0 when container doesn't exist (the container itself is the missing resource)");
351+
352+
// Test 3: ReplaceContainer on non-existent container should also return 404 with substatus 0
353+
ContainerProperties containerSettings = new ContainerProperties(id: DoesNotExist, partitionKeyPath: "/pk");
354+
response = await nonExistentContainer.ReplaceContainerStreamAsync(containerSettings);
355+
356+
Assert.IsNotNull(response);
357+
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
358+
Assert.AreEqual(0, (int)response.Headers.SubStatusCode,
359+
"ReplaceContainer should return substatus 0 when container doesn't exist (the container itself is the missing resource)");
360+
361+
// Cleanup
362+
await db.DeleteAsync();
363+
}
364+
284365
private async Task VerifyQueryNotFoundResponse(FeedIterator iterator)
285366
{
286367
ResponseMessage response = await iterator.ReadNextAsync();
@@ -294,4 +375,4 @@ private void VerifyNotFoundResponse(ResponseMessage response)
294375
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
295376
}
296377
}
297-
}
378+
}

0 commit comments

Comments
 (0)