feat(idempotency): Add function wrapper and decorator #1262
Conversation
…, and json response
… add logic to call the idempotency hander
Origin/idempotency core
github-actions
bot
added
the
feature
|
Thanks for opening this PR @KevenFuentes9! Great stuff :) |
| @@ -10,8 +10,23 @@ class IdempotencyInvalidStatusError extends Error { | |||
|
|
|||
| } | |||
|
|
|||
| class IdempotencyInconsistentStateError extends Error { | |||
|
|
|||
There was a problem hiding this comment.
Minor: adding a meaningful and easy to understand error message for each of these Errors would improve the DX and help troubleshooting errors
There was a problem hiding this comment.
I added a message to this when the error is created since this error can happen in more than one type of scenario. That way we can tailor the message by the scenario
There was a problem hiding this comment.
I left a few comments in the PR. I am happy to merge the PR as it is with the goal of accelerating progress, but I'd also like to discuss the comments I left, agree on the outcome, and ideally address them on the next steps if that is ok with you.
Let's sync on those and define a plan of action.
|
|
||
| export class IdempotencyHandler<U> { | ||
|
|
||
| public constructor(private functiontoMakeIdempotent: AnyFunctionWithRecord<U>, private functionPayloadToBeHashed: unknown, |
There was a problem hiding this comment.
Suggestion for this constructor signature:
Rename functiontoMakeIdempotent to functionToMakeIdempotent
Question: why the explicit access modifiers in the parameters? (private)
| } | ||
| } | ||
|
|
||
| public async process_idempotency(): Promise<U> { |
There was a problem hiding this comment.
Can you use camelCase for consistency?
| classWithFunction.testing(inputRecord); | ||
| }); | ||
|
|
||
| test('Then it will save the record to IN_PROGRESS', () => { |
There was a problem hiding this comment.
Suggestion: I see that in some places we use the string "IN_PROGRESS" and in others "INPROGRESS". I am in favour of the former for readability, but most importantly we should stick to one.
Description of your changes
This PR includes the creation of the function wrapper as well as the decorator for the idempotency package, creating a wrapped function that will invoke the idempotency logic. This does not yet address the saving of the record into the persistence layer, but more so has the concurrently logic underlying the idempotency logic. The record saving/retrieving will be done in a subsequent PR. Once that is done, the wrapper and decorator will have the simple case Idempotency logic completed.
How to verify this change
Related issues, RFCs
Issue number: 447
PR status
Is this ready for review?: NO
Is it a breaking change?: NO
Checklist
Breaking change checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.