Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(idempotency): preserve original error when wrapping into IdempotencyPersistenceLayerError #1552

Merged
merged 3 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/idempotency/src/IdempotencyHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
IdempotencyInconsistentStateError,
IdempotencyItemAlreadyExistsError,
IdempotencyPersistenceLayerError,
} from './Exceptions';
} from './errors';
import { BasePersistenceLayer, IdempotencyRecord } from './persistence';
import { IdempotencyConfig } from './IdempotencyConfig';
import { MAX_RETRIES } from './constants';
Expand Down Expand Up @@ -80,7 +80,8 @@ export class IdempotencyHandler<U> {
);
} catch (e) {
throw new IdempotencyPersistenceLayerError(
'Failed to delete record from idempotency store'
'Failed to delete record from idempotency store',
e as Error
);
}
throw e;
Expand All @@ -92,7 +93,8 @@ export class IdempotencyHandler<U> {
);
} catch (e) {
throw new IdempotencyPersistenceLayerError(
'Failed to update success record to idempotency store'
'Failed to update success record to idempotency store',
e as Error
);
}

Expand Down Expand Up @@ -153,7 +155,10 @@ export class IdempotencyHandler<U> {
idempotencyRecord
) as U;
} else {
throw new IdempotencyPersistenceLayerError();
throw new IdempotencyPersistenceLayerError(
'Failed to save record in progress',
e as Error
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,17 @@ class IdempotencyInconsistentStateError extends Error {}
/**
* Unrecoverable error from the data store
*/
class IdempotencyPersistenceLayerError extends Error {}
class IdempotencyPersistenceLayerError extends Error {
public readonly cause: Error | undefined;

public constructor(message: string, cause?: Error) {
const errorMessage = cause
? `${message}. This error was caused by: ${cause.message}.`
: message;
super(errorMessage);
this.cause = cause;
}
}

/**
* Payload does not contain an idempotent key
Expand Down
2 changes: 1 addition & 1 deletion packages/idempotency/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * from './Exceptions';
export * from './errors';
export * from './IdempotencyConfig';
export * from './idempotentDecorator';
export * from './makeFunctionIdempotent';
11 changes: 7 additions & 4 deletions packages/idempotency/src/middleware/makeHandlerIdempotent.ts
Copy link
Contributor

@dreamorosi dreamorosi Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add the same cause related logic to the before, after, and onError phases of the middleware?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agh, good catch!

Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
IdempotencyInconsistentStateError,
IdempotencyItemAlreadyExistsError,
IdempotencyPersistenceLayerError,
} from '../Exceptions';
} from '../errors';
import { IdempotencyRecord } from '../persistence';
import { MAX_RETRIES } from '../constants';
import type {
Expand Down Expand Up @@ -124,7 +124,8 @@ const makeHandlerIdempotent = (
}
} else {
throw new IdempotencyPersistenceLayerError(
'Failed to save in progress record to idempotency store'
'Failed to save in progress record to idempotency store',
error as Error
);
}
}
Expand All @@ -149,7 +150,8 @@ const makeHandlerIdempotent = (
);
} catch (e) {
throw new IdempotencyPersistenceLayerError(
'Failed to update success record to idempotency store'
'Failed to update success record to idempotency store',
e as Error
);
}
};
Expand All @@ -172,7 +174,8 @@ const makeHandlerIdempotent = (
);
} catch (error) {
throw new IdempotencyPersistenceLayerError(
'Failed to delete record from idempotency store'
'Failed to delete record from idempotency store',
error as Error
);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { BasePersistenceLayerInterface } from './BasePersistenceLayerInterface';
import {
IdempotencyItemAlreadyExistsError,
IdempotencyValidationError,
} from '../Exceptions';
} from '../errors';
import { LRUCache } from './LRUCache';

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
IdempotencyItemAlreadyExistsError,
IdempotencyItemNotFoundError,
} from '../Exceptions';
} from '../errors';
import { IdempotencyRecordStatus } from '../types';
import type { DynamoPersistenceOptions } from '../types';
import {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { IdempotencyRecordOptions } from '../types';
import { IdempotencyRecordStatus } from '../types';
import { IdempotencyInvalidStatusError } from '../Exceptions';
import { IdempotencyInvalidStatusError } from '../errors';

/**
* Class representing an idempotency record.
Expand Down
16 changes: 11 additions & 5 deletions packages/idempotency/tests/unit/IdempotencyHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
IdempotencyInconsistentStateError,
IdempotencyItemAlreadyExistsError,
IdempotencyPersistenceLayerError,
} from '../../src/Exceptions';
} from '../../src/errors';
import { IdempotencyRecordStatus } from '../../src/types';
import { BasePersistenceLayer, IdempotencyRecord } from '../../src/persistence';
import { IdempotencyHandler } from '../../src/IdempotencyHandler';
Expand Down Expand Up @@ -165,16 +165,20 @@ describe('Class IdempotencyHandler', () => {
});

test('when persistences store throws any error, it wraps the error to IdempotencyPersistencesLayerError', async () => {
const innerError = new Error('Some error');
const mockSaveInProgress = jest
.spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress')
.mockRejectedValue(new Error('Some error'));
.mockRejectedValue(innerError);
const mockDetermineResultFromIdempotencyRecord = jest
.spyOn(IdempotencyHandler, 'determineResultFromIdempotencyRecord')
.mockImplementation(() => 'result');

await expect(idempotentHandler.processIdempotency()).rejects.toThrow(
IdempotencyPersistenceLayerError
new IdempotencyPersistenceLayerError(
'Failed to save record in progress',
innerError
)
);

expect(mockSaveInProgress).toHaveBeenCalledTimes(1);
expect(mockDetermineResultFromIdempotencyRecord).toHaveBeenCalledTimes(0);
});
Expand Down Expand Up @@ -323,7 +327,9 @@ describe('Class IdempotencyHandler', () => {
.mockRejectedValue(new Error('Some error'));

await expect(idempotentHandler.getFunctionResult()).rejects.toThrow(
IdempotencyPersistenceLayerError
new IdempotencyPersistenceLayerError(
'Failed to delete record from idempotency store. This error was caused by: Some error.'
)
);
expect(mockDeleteInProgress).toHaveBeenCalledTimes(1);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
IdempotencyInconsistentStateError,
IdempotencyItemAlreadyExistsError,
IdempotencyPersistenceLayerError,
} from '../../src/Exceptions';
} from '../../src/errors';
import { IdempotencyConfig } from '../../src';
import { Context } from 'aws-lambda';
import { helloworldContext } from '@aws-lambda-powertools/commons/lib/samples/resources/contexts';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
IdempotencyInconsistentStateError,
IdempotencyItemAlreadyExistsError,
IdempotencyPersistenceLayerError,
} from '../../src/Exceptions';
} from '../../src/errors';
import { IdempotencyConfig } from '../../src';
import { Context } from 'aws-lambda';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
IdempotencyInconsistentStateError,
IdempotencyItemAlreadyExistsError,
IdempotencyPersistenceLayerError,
} from '../../src/Exceptions';
} from '../../src/errors';
import { IdempotencyConfig } from '../../src/';
import middy from '@middy/core';
import { MAX_RETRIES } from '../../src/constants';
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('Middleware: makeHandlerIdempotent', () => {
// Act && Assess
await expect(handler(event, context)).rejects.toThrowError(
new IdempotencyPersistenceLayerError(
'Failed to save in progress record to idempotency store'
'Failed to save in progress record to idempotency store. This error was caused by: Something went wrong.'
)
);
});
Expand All @@ -131,7 +131,7 @@ describe('Middleware: makeHandlerIdempotent', () => {
// Act && Assess
await expect(handler(event, context)).rejects.toThrowError(
new IdempotencyPersistenceLayerError(
'Failed to update success record to idempotency store'
'Failed to update success record to idempotency store. This error was caused by: Something went wrong.'
)
);
});
Expand All @@ -149,7 +149,7 @@ describe('Middleware: makeHandlerIdempotent', () => {
// Act && Assess
await expect(handler(event, context)).rejects.toThrow(
new IdempotencyPersistenceLayerError(
'Failed to delete record from idempotency store'
'Failed to delete record from idempotency store. This error was caused by: Something went wrong.'
)
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import {
IdempotencyItemAlreadyExistsError,
IdempotencyValidationError,
} from '../../../src/Exceptions';
} from '../../../src/errors';
import type { IdempotencyConfigOptions } from '../../../src/types';
import { IdempotencyRecordStatus } from '../../../src/types';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { DynamoDBPersistenceLayer } from '../../../src/persistence/DynamoDBPersi
import {
IdempotencyItemAlreadyExistsError,
IdempotencyItemNotFoundError,
} from '../../../src/Exceptions';
} from '../../../src/errors';
import { IdempotencyRecord } from '../../../src/persistence';
import type { DynamoPersistenceOptions } from '../../../src/types';
import { IdempotencyRecordStatus } from '../../../src/types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* @group unit/idempotency/persistence/idempotencyRecord
*/
import { IdempotencyInvalidStatusError } from '../../../src/Exceptions';
import { IdempotencyInvalidStatusError } from '../../../src/errors';
import { IdempotencyRecord } from '../../../src/persistence';
import { IdempotencyRecordStatus } from '../../../src/types';

Expand Down