-
Notifications
You must be signed in to change notification settings - Fork 1
PER-10200 Web app display error on multi part uploads #574
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
base: main
Are you sure you want to change the base?
Conversation
b828fe7
to
b127429
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #574 +/- ##
==========================================
+ Coverage 44.50% 44.74% +0.24%
==========================================
Files 369 369
Lines 11279 11283 +4
Branches 1849 1850 +1
==========================================
+ Hits 5020 5049 +29
+ Misses 6088 6067 -21
+ Partials 171 167 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8efc98e
to
06f34cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused -- I see one change to the the uploader logic which forcefully overrides the type of the return value registerMultipartRecord
and removes a call to getRecordVO()
.
Does registerMultipartRecord
's promise truly return a RecordVO
instead of the documented RecordResponse
?
If so, we need to update the type definition.
What was the runtime issue that was resulting in the original error and how does this resolve that? What changed that causes response
to be a recordVO? Does it ALWAYS return a recordVO directly or only in the case of a multipart upload?
@slifty changed the function, but i dont really know what to do about that response in the uploader, because it s looking for a RecordVO |
Fix response from multi part upload
f3dd0ff
to
a33799e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great thank you!
And thank you again for the pairing this morning to talk through the typing.
Steps to test: