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

🐛(backend) submit for signature handle timeout and exception on delete signing procedure #924

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonathanreveille
Copy link
Member

@jonathanreveille jonathanreveille commented Sep 13, 2024

Purpose

When the method submit_for_signature needs to delete a signing procedure because some elements have changed overtime in the contract definition and before the student signed the document, it appears that it may take a while on the signature provider side to execute the deletion.

In order to handle and update our contract, we have added a try except block to catch the timeout issue, and proceed to update the contract as if we would have received the response from the signature provider.

Before we added the fix, the contract would keep the outdated references that would not work when the student wanted to sign the document, causing another error because it has already been deleted after the timeout we have set in the
signature backend on the signature provider's side.
Now, when get a NOT_FOUND status code on the response when attempting to delete the outdated reference at the signature provider. We now make sure that handle this case and reset the contrat for the next submission.

Proposal

  • Add try except block in submit_for_signature to handle BackendTimeout and NOT_FOUND status code

CHANGELOG.md Outdated Show resolved Hide resolved
src/backend/joanie/core/exceptions.py Outdated Show resolved Hide resolved
src/backend/joanie/core/utils/signature.py Outdated Show resolved Hide resolved
@jonathanreveille jonathanreveille force-pushed the bug/order_delete_signing_procedure_catch_exceptions branch from 4b4374c to 6d8744a Compare September 16, 2024 13:46
Copy link

@wilbrdt wilbrdt left a comment

Choose a reason for hiding this comment

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

I'll let the others review as I'm still new to Joanie, but apart from two additional suggestions it looks good to me!

src/backend/joanie/core/exceptions.py Outdated Show resolved Hide resolved
src/backend/joanie/signature/backends/lex_persona.py Outdated Show resolved Hide resolved
@jonathanreveille jonathanreveille force-pushed the bug/order_delete_signing_procedure_catch_exceptions branch 2 times, most recently from 61202a0 to a19f4ae Compare September 16, 2024 14:15
@jonathanreveille jonathanreveille force-pushed the bug/order_delete_signing_procedure_catch_exceptions branch 2 times, most recently from d55904b to 757573f Compare September 18, 2024 14:54
@jonathanreveille jonathanreveille force-pushed the bug/order_delete_signing_procedure_catch_exceptions branch 3 times, most recently from 3050c11 to f45d8f1 Compare September 18, 2024 15:43
@jonathanreveille jonathanreveille force-pushed the bug/order_delete_signing_procedure_catch_exceptions branch from f45d8f1 to 6edaf3b Compare September 20, 2024 15:12
When the method `submit_for_signature` needs to delete a signing procedure
because some elements have changed overtime in the contract definition
and before the student signed the document, it appears that it may take
a while on the signature provider side to execute the deletion.
In order to handle and update our contract, we have added a try
except block to catch the timeout issue, and proceed to update the contract
as if we would have received the response from the signature provider.
Before we added the fix, the contract would keep the old reference that would
not work when the student wanted to sign the document, causing another error
because it has already been deleted after the timeout we have set in the
signature backend. In some cases, due to the timeout error, when we attempt
to delete the outdated reference, we would then have a NOT_FOUND status
code, we should also reset the contrat submission value as well to restart with
a new contrat to submit.
@jonathanreveille jonathanreveille force-pushed the bug/order_delete_signing_procedure_catch_exceptions branch from 6edaf3b to de0653e Compare September 20, 2024 15:12
Comment on lines +544 to +546
raise BackendTimeout(
f"Deletion request is taking longer than expected for reference: {reference_id}"
) from exception
Copy link
Member

Choose a reason for hiding this comment

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

By reading that, I feel BackendTimeout exception duplicates the ReadTimeout exception. We could simply raise the received exception.

Suggested change
raise BackendTimeout(
f"Deletion request is taking longer than expected for reference: {reference_id}"
) from exception
raise exception

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants