Skip to content

Commit 661f5ff

Browse files
dreamorosidreamorosi
authored
fix(idempotency): include sk in error msgs when using composite key (#3709)
1 parent cab716d commit 661f5ff

File tree

6 files changed

+149
-56
lines changed

6 files changed

+149
-56
lines changed

Diff for: ‎packages/idempotency/src/IdempotencyHandler.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,11 @@ export class IdempotencyHandler<Func extends AnyFunction> {
120120
);
121121
}
122122
throw new IdempotencyAlreadyInProgressError(
123-
`There is already an execution in progress with idempotency key: ${idempotencyRecord.idempotencyKey}`
123+
`There is already an execution in progress with idempotency key: ${idempotencyRecord.idempotencyKey}${
124+
idempotencyRecord.sortKey
125+
? ` and sort key: ${idempotencyRecord.sortKey}`
126+
: ''
127+
}`
124128
);
125129
}
126130

Diff for: ‎packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer {
122122

123123
return new IdempotencyRecord({
124124
idempotencyKey: item[this.keyAttr],
125+
sortKey: this.sortKeyAttr && item[this.sortKeyAttr],
125126
status: item[this.statusAttr],
126127
expiryTimestamp: item[this.expiryAttr],
127128
inProgressExpiryTimestamp: item[this.inProgressExpiryAttr],
@@ -207,14 +208,17 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer {
207208
item &&
208209
new IdempotencyRecord({
209210
idempotencyKey: item[this.keyAttr],
211+
sortKey: this.sortKeyAttr && item[this.sortKeyAttr],
210212
status: item[this.statusAttr],
211213
expiryTimestamp: item[this.expiryAttr],
212214
inProgressExpiryTimestamp: item[this.inProgressExpiryAttr],
213215
responseData: item[this.dataAttr],
214216
payloadHash: item[this.validationKeyAttr],
215217
});
216218
throw new IdempotencyItemAlreadyExistsError(
217-
`Failed to put record for already existing idempotency key: ${record.idempotencyKey}`,
219+
`Failed to put record for already existing idempotency key: ${record.idempotencyKey}${
220+
this.sortKeyAttr ? ` and sort key: ${record.sortKey}` : ''
221+
}`,
218222
idempotencyRecord
219223
);
220224
}

Diff for: ‎packages/idempotency/src/persistence/IdempotencyRecord.ts

+6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type {
55
IdempotencyRecordOptions,
66
IdempotencyRecordStatusValue,
77
} from '../types/IdempotencyRecord.js';
8+
import type { DynamoDBPersistenceLayer } from './DynamoDBPersistenceLayer.js';
89

910
/**
1011
* Class representing an idempotency record.
@@ -19,6 +20,10 @@ class IdempotencyRecord {
1920
* The idempotency key of the record that is used to identify the record.
2021
*/
2122
public idempotencyKey: string;
23+
/**
24+
* An optional sort key that can be used with the {@link DynamoDBPersistenceLayer | `DynamoDBPersistenceLayer`}.
25+
*/
26+
public sortKey?: string;
2227
/**
2328
* The expiry timestamp of the in progress record in milliseconds UTC.
2429
*/
@@ -46,6 +51,7 @@ class IdempotencyRecord {
4651
this.responseData = config.responseData;
4752
this.payloadHash = config.payloadHash;
4853
this.status = config.status;
54+
this.sortKey = config.sortKey;
4955
}
5056

5157
/**

Diff for: ‎packages/idempotency/src/types/IdempotencyRecord.ts

+24
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,35 @@ type IdempotencyRecordStatusValue =
1111
* Options for creating a new IdempotencyRecord
1212
*/
1313
type IdempotencyRecordOptions = {
14+
/**
15+
* The idempotency key of the record that is used to identify the record.
16+
*/
1417
idempotencyKey: string;
18+
/**
19+
* An optional sort key that can be used with the {@link DynamoDBPersistenceLayer | `DynamoDBPersistenceLayer`}.
20+
*/
21+
sortKey?: string;
22+
/**
23+
* The idempotency record status can be COMPLETED, IN_PROGRESS or EXPIRED.
24+
* We check the status during idempotency processing to make sure we don't process an expired record and handle concurrent requests.
25+
* {@link constants.IdempotencyRecordStatusValue | IdempotencyRecordStatusValue}
26+
*/
1527
status: IdempotencyRecordStatusValue;
28+
/**
29+
* The expiry timestamp of the record in milliseconds UTC.
30+
*/
1631
expiryTimestamp?: number;
32+
/**
33+
* The expiry timestamp of the in progress record in milliseconds UTC.
34+
*/
1735
inProgressExpiryTimestamp?: number;
36+
/**
37+
* The response data of the request, this will be returned if the payload hash matches.
38+
*/
1839
responseData?: JSONValue;
40+
/**
41+
* The hash of the payload of the request, used for comparing requests.
42+
*/
1943
payloadHash?: string;
2044
};
2145

Diff for: ‎packages/idempotency/tests/unit/IdempotencyHandler.test.ts

+40-19
Original file line numberDiff line numberDiff line change
@@ -49,25 +49,46 @@ describe('Class IdempotencyHandler', () => {
4949
});
5050

5151
describe('Method: determineResultFromIdempotencyRecord', () => {
52-
it('throws when the record is in progress and within expiry window', async () => {
53-
// Prepare
54-
const stubRecord = new IdempotencyRecord({
55-
idempotencyKey: 'idempotencyKey',
56-
expiryTimestamp: Date.now() + 1000, // should be in the future
57-
inProgressExpiryTimestamp: 0, // less than current time in milliseconds
58-
responseData: { responseData: 'responseData' },
59-
payloadHash: 'payloadHash',
60-
status: IdempotencyRecordStatus.INPROGRESS,
61-
});
62-
63-
// Act & Assess
64-
expect(stubRecord.isExpired()).toBe(false);
65-
expect(stubRecord.getStatus()).toBe(IdempotencyRecordStatus.INPROGRESS);
66-
expect(() =>
67-
idempotentHandler.determineResultFromIdempotencyRecord(stubRecord)
68-
).toThrow(IdempotencyAlreadyInProgressError);
69-
expect(mockResponseHook).not.toHaveBeenCalled();
70-
});
52+
it.each([
53+
{
54+
keys: {
55+
idempotencyKey: 'idempotencyKey',
56+
sortKey: 'sk',
57+
},
58+
expectedErrorMsg:
59+
'There is already an execution in progress with idempotency key: idempotencyKey and sort key: sk',
60+
case: 'pk & sk',
61+
},
62+
{
63+
keys: {
64+
idempotencyKey: 'idempotencyKey',
65+
},
66+
expectedErrorMsg:
67+
'There is already an execution in progress with idempotency key: idempotencyKey',
68+
case: 'pk only',
69+
},
70+
])(
71+
'throws when the record is in progress and within expiry window ($case)',
72+
async ({ keys, expectedErrorMsg }) => {
73+
// Prepare
74+
const stubRecord = new IdempotencyRecord({
75+
...keys,
76+
expiryTimestamp: Date.now() + 1000, // should be in the future
77+
inProgressExpiryTimestamp: 0, // less than current time in milliseconds
78+
responseData: { responseData: 'responseData' },
79+
payloadHash: 'payloadHash',
80+
status: IdempotencyRecordStatus.INPROGRESS,
81+
});
82+
83+
// Act & Assess
84+
expect(stubRecord.isExpired()).toBe(false);
85+
expect(stubRecord.getStatus()).toBe(IdempotencyRecordStatus.INPROGRESS);
86+
expect(() =>
87+
idempotentHandler.determineResultFromIdempotencyRecord(stubRecord)
88+
).toThrow(new IdempotencyAlreadyInProgressError(expectedErrorMsg));
89+
expect(mockResponseHook).not.toHaveBeenCalled();
90+
}
91+
);
7192

7293
it('throws when the record is in progress and outside expiry window', async () => {
7394
// Prepare

Diff for: ‎packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts

+69-35
Original file line numberDiff line numberDiff line change
@@ -346,41 +346,75 @@ describe('Class: DynamoDBPersistenceLayer', () => {
346346
persistenceLayerSpy.mockRestore();
347347
});
348348

349-
it('throws when called with a record that fails any condition', async () => {
350-
// Prepare
351-
const record = new IdempotencyRecord({
352-
idempotencyKey: dummyKey,
353-
status: IdempotencyRecordStatus.EXPIRED,
354-
expiryTimestamp: 0,
355-
});
356-
const expiration = Date.now();
357-
client.on(PutItemCommand).rejects(
358-
new ConditionalCheckFailedException({
359-
$metadata: {
360-
httpStatusCode: 400,
361-
requestId: 'someRequestId',
362-
},
363-
message: 'Conditional check failed',
364-
Item: {
365-
id: { S: 'test-key' },
366-
status: { S: 'INPROGRESS' },
367-
expiration: { N: expiration.toString() },
368-
},
369-
})
370-
);
371-
372-
// Act & Assess
373-
await expect(persistenceLayer._putRecord(record)).rejects.toThrowError(
374-
new IdempotencyItemAlreadyExistsError(
375-
`Failed to put record for already existing idempotency key: ${record.idempotencyKey}`,
376-
new IdempotencyRecord({
377-
idempotencyKey: 'test-key',
378-
status: IdempotencyRecordStatus.INPROGRESS,
379-
expiryTimestamp: expiration,
349+
it.each([
350+
{
351+
keys: {
352+
id: 'idempotency#my-lambda-function',
353+
sortKey: dummyKey,
354+
},
355+
case: 'composite key',
356+
},
357+
{
358+
keys: {
359+
id: dummyKey,
360+
},
361+
case: 'single key',
362+
},
363+
])(
364+
'throws when called with a record that fails any condition ($case)',
365+
async ({ keys }) => {
366+
// Prepare
367+
const { id, sortKey } = keys;
368+
369+
const record = new IdempotencyRecord({
370+
idempotencyKey: id,
371+
sortKey,
372+
status: IdempotencyRecordStatus.EXPIRED,
373+
expiryTimestamp: 0,
374+
});
375+
const expiration = Date.now();
376+
client.on(PutItemCommand).rejects(
377+
new ConditionalCheckFailedException({
378+
$metadata: {
379+
httpStatusCode: 400,
380+
requestId: 'someRequestId',
381+
},
382+
message: 'Conditional check failed',
383+
Item: {
384+
id: { S: 'test-key' },
385+
...(sortKey ? { sortKey: { S: sortKey } } : {}),
386+
status: { S: 'INPROGRESS' },
387+
expiration: { N: expiration.toString() },
388+
},
380389
})
381-
)
382-
);
383-
});
390+
);
391+
const testPersistenceLayer = sortKey
392+
? new DynamoDBPersistenceLayerTestClass({
393+
tableName: dummyTableName,
394+
sortKeyAttr: 'sortKey',
395+
})
396+
: persistenceLayer;
397+
398+
// Act & Assess
399+
await expect(
400+
testPersistenceLayer._putRecord(record)
401+
).rejects.toThrowError(
402+
new IdempotencyItemAlreadyExistsError(
403+
`Failed to put record for already existing idempotency key: ${
404+
sortKey
405+
? `${record.idempotencyKey} and sort key: ${sortKey}`
406+
: record.idempotencyKey
407+
}`,
408+
new IdempotencyRecord({
409+
idempotencyKey: 'test-key',
410+
sortKey,
411+
status: IdempotencyRecordStatus.INPROGRESS,
412+
expiryTimestamp: expiration,
413+
})
414+
)
415+
);
416+
}
417+
);
384418

385419
it('throws when encountering an unknown error', async () => {
386420
// Prepare
@@ -445,7 +479,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
445479
);
446480
});
447481

448-
it('it builds the request correctly when using composite keys', async () => {
482+
it('builds the request correctly when using composite keys', async () => {
449483
// Prepare
450484
const persistenceLayer = new DynamoDBPersistenceLayerTestClass({
451485
tableName: dummyTableName,

0 commit comments

Comments
 (0)