refactor: remove try-catch from submissions pipeline #1315
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liangyuanruo
approved these changes
Mar 8, 2021
There was a problem hiding this comment.
LGTM, thanks.
I replaced all instances of the createEmailData function with a call to new SubmissionEmailObj
They would have to be updated eventually since the construction of parsedResponses is being moved into SubmissionEmailObj as the 2nd part of @tshuli 's refactor.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We previously inserted two try-catch blocks in the submissions pipeline as the
answerfield on some responses wasundefineddue to an unknown bug. We later found that the bug was due to missing SingPass/CorpPass cookies, and the bug was fixed in #918.As shown in the following queries in CloudWatch, we have had no instances of these errors in the past 4 weeks:
Closes #847
Solution
First, the try-catch blocks were removed. Then I noticed that the function
createEmailDatawas just a dumb wrapper aroundSubmissionEmailObj, so I replaced all instances of thecreateEmailDatafunction with a call tonew SubmissionEmailObj. SincecreateEmailDatahad extensive unit tests, I simply changed all its unit tests to callnew SubmissionEmailObjinstead (see diagram below), and found that the tests still passed as expected.Tests