Companion+client stability fixes, error handling and retry #4734
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
400 seems inappropriate
respond with correct response code when calling /get and request to the upstream server fail also add a unit test for this
allows us to retry it
- Make sure we don't instantiate a new Provider for every file uploaded (reuse the original provider instead) - this caused the refresh token synchronization not to work - Retry most errors when uploading (`/get`) using companion, but for HTTP, only retry certain HTTP status codes - Reimplement the websocket code to be more stable and retry as well
because it isn't supported in Companion as far as I can tell
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Murderlon
reviewed
Oct 19, 2023
| }) | ||
| if (!skipPostResponse) this.onReceiveResponse(response) | ||
| return handleJSONResponse(response) | ||
| return await pRetry(async () => { |
There was a problem hiding this comment.
Just a head up that is will be need to be refactored again in #4745, as we now have a util for fetching and retrying.
or else we risk getting stuck retrying just half of the operation, which might never recover
Murderlon
reviewed
Oct 23, 2023
|
CI is also failing and should be looked at |
- rewrite so we can use p-retry for exponential backoff for socket reconnect too - improve logging - simplify/reuse code
|
pushed some improvements. e2e tests are back in business. i'm no longer retrying all http requests (as before, which caused the e2e to fail) |
aduh95
reviewed
Oct 23, 2023
aduh95
reviewed
Oct 23, 2023
Co-authored-by: Antoine du Hamel <[email protected]>
mifi
commented
Oct 24, 2023
Co-authored-by: Mikael Finstad <[email protected]>
mifi
commented
Oct 25, 2023
|
one e2e test flaked: should not create assembly when all individual files have been cancelled |
Murderlon
reviewed
Oct 26, 2023
for google drive
and event handler error catching
if a file is canceled
Murderlon
approved these changes
Oct 31, 2023
arturi
approved these changes
Nov 1, 2023
github-actions bot
added a commit
that referenced
this pull request
Nov 8, 2023
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 3.5.0 | @uppy/provider-views | 3.7.0 | | @uppy/aws-s3-multipart | 3.9.0 | @uppy/react | 3.2.0 | | @uppy/companion | 4.11.0 | @uppy/transloadit | 3.4.0 | | @uppy/companion-client | 3.6.0 | @uppy/tus | 3.4.0 | | @uppy/core | 3.7.0 | @uppy/url | 3.4.0 | | @uppy/dashboard | 3.7.0 | @uppy/utils | 5.6.0 | | @uppy/image-editor | 2.3.0 | @uppy/xhr-upload | 3.5.0 | | @uppy/locales | 3.4.0 | uppy | 3.19.0 | - @uppy/dashboard: Remove uppy-Dashboard-isFixed when uppy.close() is invoked (Artur Paikin / #4775) - @uppy/core,@uppy/dashboard: don't cancel all files when clicking "done" (Mikael Finstad / #4771) - @uppy/utils: refactor to TS (Antoine du Hamel / #4699) - @uppy/locales: locales: add ca_ES (ordago / #4772) - @uppy/companion: Companion+client stability fixes, error handling and retry (Mikael Finstad / #4734) - @uppy/companion: add getBucket metadata argument (Mikael Finstad / #4770) - @uppy/core: simplify types with class generic (JokcyLou / #4761) - @uppy/image-editor: More image editor improvements (Evgenia Karunus / #4676) - @uppy/react: add useUppyState (Merlijn Vos / #4711)
2 tasks
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.
400 seems inappropriate
allows us to retry it
refresh-tokencalls to be made concurrently, possibly causing problems/get) using companion, but for HTTP, only retry certain HTTP status codes