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.

Sign up for GitHub

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

Jump to bottom

chore: log errors from concatResponse #817

Merged
merged 2 commits into from
Dec 9, 2020
Merged

chore: log errors from concatResponse #817

merged 2 commits into from
Dec 9, 2020

Conversation

tshuli
Copy link
Contributor

@tshuli tshuli commented Dec 8, 2020 •
edited
Loading

(to rebase after #755)

Problem

Undefined answers are being passed into submission
Part of #808

Solution

  • Not immediately apparent where the validation gap is as unable to replicate
  • Propose to log errors from concatResponse first so that we have more details to replicate and debug
  • TODO: Review logs in 1 weeks' time

@mantariksh
Copy link
Contributor

conflicts with #755 fyi

@tshuli tshuli force-pushed the fix/concat-response branch from 5872150 to f731a32 Compare December 8, 2020 04:13
liangyuanruo
liangyuanruo suggested changes Dec 8, 2020
View reviewed changes
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

the code should return a response after the log statement, otherwise concatenatedResponse will continue processing with an undefined value.

@tshuli tshuli force-pushed the fix/concat-response branch from f731a32 to 7e6fed6 Compare December 8, 2020 12:56
@tshuli tshuli requested a review from liangyuanruo December 8, 2020 12:59
@tshuli
Copy link
Contributor Author

tshuli commented Dec 9, 2020

@liangyuanruo for approval

@liangyuanruo liangyuanruo merged commit bb4736c into develop Dec 9, 2020
@liangyuanruo liangyuanruo deleted the fix/concat-response branch December 9, 2020 09:15
mantariksh pushed a commit that referenced this pull request Dec 9, 2020
@tshuli @mantariksh
chore: log errors from concatResponse (#817)
da7bc45
@mantariksh mantariksh mentioned this pull request Dec 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mantariksh mantariksh mantariksh approved these changes

@liangyuanruo liangyuanruo liangyuanruo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tshuli @mantariksh @liangyuanruo