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

@uppy/xhr-upload: align options with tus #5049

Closed
2 tasks done
Tracked by #4764
Murderlon opened this issue Mar 28, 2024 · 1 comment
Closed
2 tasks done
Tracked by #4764

@uppy/xhr-upload: align options with tus #5049

Murderlon opened this issue Mar 28, 2024 · 1 comment
Assignees
Labels
4.0 For the 4.0 major version Feature

Comments

@Murderlon
Copy link
Member

Murderlon commented Mar 28, 2024

Initial checklist

  • I understand this is a feature request and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Problem

There are two problems.

  1. xhr-upload has options getResponseData, getResponseError, and responseUrlFieldName to extract data for local files, these options are not used for responses from Companion as they are handled inside companion-client. Those options where introduced for people who want to convert XML to JSON (can't believe we support that 🙈). Funnily enough, those people with XML were never able to use Companion as Companion expects the response to be JSON before sending it over websockets. Besides that these options also feel awkward and could all be replaced with a single onAfterReponse.
  2. There is no good way to refresh an auth token when it expires: Support async upload parameters on XHR upload #5042

Solution

We need two changes.

  1. Make onShouldRetry in tus-js-client support promises. At the moment when you want to refresh a token with tus you have to duplicate logic in onShouldRetry and onAfterResponse:
import Uppy from '@uppy/core';
import Tus from '@uppy/tus';

new Uppy().use(Tus, {
  endpoint: '',
  async onBeforeRequest(req) {
    const token = await getAuthToken();
    req.setHeader('Authorization', `Bearer ${token}`);
  },
  onShouldRetry(err, retryAttempt, options, next) {
    if (err?.originalResponse?.getStatus() === 401) {
      return true;
    }
    return next(err);
  },
  async onAfterResponse(req, res) {
    if (res.getStatus() === 401) {
      await refreshAuthToken();
    }
  },
});

If onShouldRetry supports promises, we can put the await refreshAuthToken() before the return true and skip onAfterResponse.

  1. Remove getResponseData, getResponseError, validateStatus, and responseUrlFieldName in favor of tus-style callbacks onBeforeRequest, onShouldRetry, and onAfterResponse, and optionally onError.

Alternatives

🤷

@Murderlon
Copy link
Member Author

Closed in #5094

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.0 For the 4.0 major version Feature
Projects
None yet
Development

No branches or pull requests

1 participant