Upload: tab sync and various fixes (#428)

* Upload: fix redux key clash

## Issue
`params` is the "final" value that will be passed to the SDK and  `channel` is not a valid argument (it should be `channel_name`). Also, it seems like we only pass the channel ID now and skip the channel name entirely.

For the anonymous case, a clash will still happen when since the channel part is hardcoded to `anonymous`.

## Approach
Generate a guid in `params` and use that as the key to handle all the cases above. We couldn't use the `uploadUrl` because v1 doesn't have it.

The old formula is retained to allow users to retry or cancel their existing uploads one last time (otherwise it will persist forever). The next upload will be using the new key.

* Upload: add tab-locking

## Issue
- The previous code does detect uploads from multiple tabs, but it was done by handling the CONFLICT error message from the backend. At certain corner-cases, this does not work well. A better way is to not allow resumption while the same file is being uploading from another tab.

- When an upload from 1 tab finishes, the GUI on the other tab does not remove the completed item. User either have to refresh or click Cancel. Clicking Cancel results in the 404 backend error. This should be avoided.

## Approach
- Added tab synchronization and locking by passing the "locked" and "removed" information through `localStorage`.

## Other considered approaches
- Wallet sync -- but decided not to pollute the wallet.
- 3rd-party redux tab syncing -- but decided it's not worth adding another module for 1 usage.

* Upload: check if locked before confirming delete

## Reproduce
Have 2 tabs + paused upload
Open "cancel" dialog in one of the tabs.
Continue upload in other tab
Confirm cancellation in first tab
Upload disappears from both tabs, but based on network traffic the upload keeps happening.
(If upload finishes the claim seems to get created)
This commit is contained in:
infinite-persistence 2021-12-07 06:48:09 -08:00 committed by GitHub
parent e982d9c13c
commit 157b50c58e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 286 additions and 58 deletions

View file

@ -62,7 +62,8 @@ declare type FileUploadSdkParams = {
remote_url?: string,
thumbnail_url?: string,
title?: string,
// Temporary values
// Temporary values; remove when passing to SDK
guid: string,
uploadUrl?: string,
};

View file

@ -1307,6 +1307,8 @@
"Uploading...": "Uploading...",
"Creating...": "Creating...",
"Stopped. Duplicate session detected.": "Stopped. Duplicate session detected.",
"File being uploaded in another tab or window.": "File being uploaded in another tab or window.",
"There are pending uploads.": "There are pending uploads.",
"Use a URL": "Use a URL",
"Edit Cover Image": "Edit Cover Image",
"Cover Image": "Cover Image",

View file

@ -2,6 +2,7 @@
import * as PAGES from 'constants/pages';
import React, { useEffect, useRef, useState, useLayoutEffect } from 'react';
import { lazyImport } from 'util/lazyImport';
import { tusUnlockAndNotify, tusHandleTabUpdates } from 'util/tus';
import classnames from 'classnames';
import analytics from 'analytics';
import { setSearchUserId } from 'redux/actions/search';
@ -231,12 +232,29 @@ function App(props: Props) {
useEffect(() => {
if (!uploadCount) return;
const handleUnload = (event) => tusUnlockAndNotify();
const handleBeforeUnload = (event) => {
event.preventDefault();
event.returnValue = 'magic'; // without setting this to something it doesn't work
event.returnValue = __('There are pending uploads.'); // without setting this to something it doesn't work
};
window.addEventListener('unload', handleUnload);
window.addEventListener('beforeunload', handleBeforeUnload);
return () => window.removeEventListener('beforeunload', handleBeforeUnload);
return () => {
window.removeEventListener('unload', handleUnload);
window.removeEventListener('beforeunload', handleBeforeUnload);
};
}, [uploadCount]);
useEffect(() => {
if (!uploadCount) return;
const onStorageUpdate = (e) => tusHandleTabUpdates(e.key);
window.addEventListener('storage', onStorageUpdate);
return () => window.removeEventListener('storage', onStorageUpdate);
}, [uploadCount]);
// allows user to pause miniplayer using the spacebar without the page scrolling down

View file

@ -5,11 +5,12 @@ import Button from 'component/button';
import FileThumbnail from 'component/fileThumbnail';
import * as MODALS from 'constants/modal_types';
import { serializeFileObj } from 'util/file';
import { tusIsSessionLocked } from 'util/tus';
type Props = {
uploadItem: FileUploadItem,
doPublishResume: (any) => void,
doUpdateUploadRemove: (any) => void,
doUpdateUploadRemove: (string, any) => void,
doOpenModal: (string, {}) => void,
};
@ -18,11 +19,16 @@ export default function WebUploadItem(props: Props) {
const { params, file, fileFingerprint, progress, status, resumable, uploader } = uploadItem;
const [showFileSelector, setShowFileSelector] = useState(false);
const locked = tusIsSessionLocked(params.guid);
function handleFileChange(newFile: WebFile, clearName = true) {
if (serializeFileObj(newFile) === fileFingerprint) {
setShowFileSelector(false);
doPublishResume({ ...params, file_path: newFile });
if (!params.guid) {
// Can remove this if-block after January 2022.
doUpdateUploadRemove('', params);
}
} else {
doOpenModal(MODALS.CONFIRM, {
title: __('Invalid file'),
@ -40,21 +46,34 @@ export default function WebUploadItem(props: Props) {
subtitle: __('Cancel and remove the selected upload?'),
body: params.name ? <p className="empty">{`lbry://${params.name}`}</p> : undefined,
onConfirm: (closeModal) => {
if (uploader) {
if (resumable) {
// $FlowFixMe - couldn't resolve to TusUploader manually.
uploader.abort(true); // TUS
} else {
uploader.abort(); // XHR
if (tusIsSessionLocked(params.guid)) {
// Corner-case: it's possible for the upload to resume in another tab
// after the modal has appeared. Make a final lock-check here.
// We can invoke a toast here, but just do nothing for now.
// The upload status should make things obvious.
} else {
if (uploader) {
if (resumable) {
// $FlowFixMe - couldn't resolve to TusUploader manually.
uploader.abort(true); // TUS
} else {
uploader.abort(); // XHR
}
}
// The second parameter (params) can be removed after January 2022.
doUpdateUploadRemove(params.guid, params);
}
doUpdateUploadRemove(params);
closeModal();
},
});
}
function resolveProgressStr() {
if (locked) {
return __('File being uploaded in another tab or window.');
}
if (!uploader) {
return __('Stopped.');
}
@ -81,7 +100,7 @@ export default function WebUploadItem(props: Props) {
}
function getRetryButton() {
if (!resumable) {
if (!resumable || locked) {
return null;
}
@ -109,7 +128,9 @@ export default function WebUploadItem(props: Props) {
}
function getCancelButton() {
return <Button label={__('Cancel')} button="link" onClick={handleCancel} />;
if (!locked) {
return <Button label={__('Cancel')} button="link" onClick={handleCancel} />;
}
}
function getFileSelector() {

View file

@ -7,7 +7,7 @@ type Props = {
currentUploads: { [key: string]: FileUploadItem },
uploadCount: number,
doPublishResume: (any) => void,
doUpdateUploadRemove: (any) => void,
doUpdateUploadRemove: (string, any) => void,
doOpenModal: (string, {}) => void,
};

3
ui/constants/storage.js Normal file
View file

@ -0,0 +1,3 @@
// Local Storage keys
export const TUS_LOCKED_UPLOADS = 'tusLockedUploads';
export const TUS_REMOVED_UPLOADS = 'tusRemovedUploads';

View file

@ -702,21 +702,28 @@ export function doUpdateUploadAdd(
};
}
export const doUpdateUploadProgress = (props: {
params: { [key: string]: any },
progress?: string,
status?: string,
}) => (dispatch: Dispatch) =>
export const doUpdateUploadProgress = (props: { guid: string, progress?: string, status?: string }) => (
dispatch: Dispatch
) =>
dispatch({
type: ACTIONS.UPDATE_UPLOAD_PROGRESS,
data: props,
});
export function doUpdateUploadRemove(params: { [key: string]: any }) {
/**
* doUpdateUploadRemove
*
* @param guid
* @param params Optional. Retain to allow removal of old keys, which are
* derived from `name#channel` instead of using a guid.
* Can be removed after January 2022.
* @returns {(function(Dispatch, GetState): void)|*}
*/
export function doUpdateUploadRemove(guid: string, params?: { [key: string]: any }) {
return (dispatch: Dispatch, getState: GetState) => {
dispatch({
type: ACTIONS.UPDATE_UPLOAD_REMOVE,
data: { params },
data: { guid, params },
});
};
}

View file

@ -2,11 +2,14 @@
import { handleActions } from 'util/redux-utils';
import { buildURI } from 'util/lbryURI';
import { serializeFileObj } from 'util/file';
import { tusLockAndNotify, tusUnlockAndNotify, tusRemoveAndNotify, tusClearRemovedUploads } from 'util/tus';
import * as ACTIONS from 'constants/action_types';
import * as THUMBNAIL_STATUSES from 'constants/thumbnail_upload_statuses';
import { CHANNEL_ANONYMOUS } from 'constants/claim';
const getKeyFromParam = (params) => `${params.name}#${params.channel || 'anonymous'}`;
// This is the old key formula. Retain it for now to allow users to delete
// any pending uploads. Can be removed from January 2022 onwards.
const getOldKeyFromParam = (params) => `${params.name}#${params.channel || 'anonymous'}`;
type PublishState = {
editingURI: ?string,
@ -138,10 +141,9 @@ export const publishReducer = handleActions(
},
[ACTIONS.UPDATE_UPLOAD_ADD]: (state: PublishState, action) => {
const { file, params, uploader } = action.data;
const key = getKeyFromParam(params);
const currentUploads = Object.assign({}, state.currentUploads);
currentUploads[key] = {
currentUploads[params.guid] = {
file,
fileFingerprint: file ? serializeFileObj(file) : undefined, // TODO: get hash instead?
progress: '0',
@ -153,10 +155,14 @@ export const publishReducer = handleActions(
return { ...state, currentUploads };
},
[ACTIONS.UPDATE_UPLOAD_PROGRESS]: (state: PublishState, action) => {
const { params, progress, status } = action.data;
const key = getKeyFromParam(params);
const { guid, progress, status } = action.data;
const key = guid;
const currentUploads = Object.assign({}, state.currentUploads);
if (guid === 'force--update') {
return { ...state, currentUploads };
}
if (!currentUploads[key]) {
return state;
}
@ -165,10 +171,16 @@ export const publishReducer = handleActions(
currentUploads[key].progress = progress;
delete currentUploads[key].status;
if (currentUploads[key].uploader.url && !currentUploads[key].params.uploadUrl) {
// TUS has finally obtained an upload url from the server. Stash that to check later when resuming.
// Ignoring immutable-update requirement (probably doesn't matter to the GUI).
currentUploads[key].params.uploadUrl = currentUploads[key].uploader.url;
if (currentUploads[key].uploader.url) {
// TUS has finally obtained an upload url from the server...
if (!currentUploads[key].params.uploadUrl) {
// ... Stash that to check later when resuming.
// Ignoring immutable-update requirement (probably doesn't matter to the GUI).
currentUploads[key].params.uploadUrl = currentUploads[key].uploader.url;
}
// ... lock this tab as the active uploader.
tusLockAndNotify(key);
}
} else if (status) {
currentUploads[key].status = status;
@ -180,13 +192,19 @@ export const publishReducer = handleActions(
return { ...state, currentUploads };
},
[ACTIONS.UPDATE_UPLOAD_REMOVE]: (state: PublishState, action) => {
const { params } = action.data;
const key = getKeyFromParam(params);
const currentUploads = Object.assign({}, state.currentUploads);
const { guid, params } = action.data;
const key = guid || getOldKeyFromParam(params);
delete currentUploads[key];
if (state.currentUploads[key]) {
const currentUploads = Object.assign({}, state.currentUploads);
delete currentUploads[key];
tusUnlockAndNotify(key);
tusRemoveAndNotify(key);
return { ...state, currentUploads };
return { ...state, currentUploads };
}
return state;
},
[ACTIONS.REHYDRATE]: (state: PublishState, action) => {
if (action && action.payload && action.payload.publish) {
@ -194,14 +212,20 @@ export const publishReducer = handleActions(
// Cleanup for 'publish::currentUploads'
if (newPublish.currentUploads) {
Object.keys(newPublish.currentUploads).forEach((key) => {
const params = newPublish.currentUploads[key].params;
if (!params || Object.keys(params).length === 0) {
delete newPublish.currentUploads[key];
} else {
delete newPublish.currentUploads[key].uploader;
}
});
const uploadKeys = Object.keys(newPublish.currentUploads);
if (uploadKeys.length > 0) {
// Clear uploader and corrupted params
uploadKeys.forEach((key) => {
const params = newPublish.currentUploads[key].params;
if (!params || Object.keys(params).length === 0) {
delete newPublish.currentUploads[key];
} else {
delete newPublish.currentUploads[key].uploader;
}
});
} else {
tusClearRemovedUploads();
}
}
return newPublish;

15
ui/util/storage.js Normal file
View file

@ -0,0 +1,15 @@
export function isLocalStorageAvailable() {
try {
return Boolean(window.localStorage);
} catch (e) {
return false;
}
}
export function isSessionStorageAvailable() {
try {
return Boolean(window.sessionStorage);
} catch (e) {
return false;
}
}

129
ui/util/tus.js Normal file
View file

@ -0,0 +1,129 @@
// @flow
/**
* This serves a bridge between tabs using localStorage to indicate whether an
* upload is currently in progress (locked) or removed.
*
* An alternative is to sync the redux's 'publish::currentUploads' through the
* wallet's sync process, but let's not pollute the wallet for now.
*/
import { v4 as uuid } from 'uuid';
import { TUS_LOCKED_UPLOADS, TUS_REMOVED_UPLOADS } from 'constants/storage';
import { isLocalStorageAvailable } from 'util/storage';
import { doUpdateUploadRemove, doUpdateUploadProgress } from 'redux/actions/publish';
const localStorageAvailable = isLocalStorageAvailable();
let gTabId: string = '';
function getTabId() {
if (!gTabId) {
// We want to maximize bootup speed, so only initialize
// the tab ID on first use instead when declared.
gTabId = uuid();
}
return gTabId;
}
// ****************************************************************************
// Locked
// ****************************************************************************
function getLockedUploads() {
if (localStorageAvailable) {
const storedValue = window.localStorage.getItem(TUS_LOCKED_UPLOADS);
return storedValue ? JSON.parse(storedValue) : {};
}
return {};
}
export function tusIsSessionLocked(guid: string) {
const lockedUploads = getLockedUploads();
return lockedUploads[guid] && lockedUploads[guid] !== getTabId();
}
export function tusLockAndNotify(guid: string) {
const lockedUploads = getLockedUploads();
if (!lockedUploads[guid] && localStorageAvailable) {
lockedUploads[guid] = getTabId();
window.localStorage.setItem(TUS_LOCKED_UPLOADS, JSON.stringify(lockedUploads));
}
}
/**
* tusUnlockAndNotify
*
* @param guid The upload session to unlock and notify other tabs of.
* Passing 'undefined' will clear all sessions locked by this tab.
*/
export function tusUnlockAndNotify(guid?: string) {
if (!localStorageAvailable) return;
const lockedUploads = getLockedUploads();
if (guid) {
delete lockedUploads[guid];
} else {
const ourTabId = getTabId();
const lockedUploadsEntries = Object.entries(lockedUploads);
lockedUploadsEntries.forEach(([lockedGuid, tabId]) => {
if (tabId === ourTabId) {
delete lockedUploads[lockedGuid];
}
});
}
if (Object.keys(lockedUploads).length > 0) {
window.localStorage.setItem(TUS_LOCKED_UPLOADS, JSON.stringify(lockedUploads));
} else {
window.localStorage.removeItem(TUS_LOCKED_UPLOADS);
}
}
// ****************************************************************************
// Removed
// ****************************************************************************
function getRemovedUploads() {
if (localStorageAvailable) {
const storedValue = window.localStorage.getItem(TUS_REMOVED_UPLOADS);
return storedValue ? storedValue.split(',') : [];
}
return [];
}
export function tusRemoveAndNotify(guid: string) {
if (!localStorageAvailable) return;
const removedUploads = getRemovedUploads();
removedUploads.push(guid);
window.localStorage.setItem(TUS_REMOVED_UPLOADS, removedUploads.join(','));
}
export function tusClearRemovedUploads() {
if (!localStorageAvailable) return;
window.localStorage.removeItem(TUS_REMOVED_UPLOADS);
}
// ****************************************************************************
// Respond to changes from other tabs.
// ****************************************************************************
export function tusHandleTabUpdates(storageKey: string) {
switch (storageKey) {
case TUS_LOCKED_UPLOADS:
// The locked IDs are in localStorage, but related GUI is unaware.
// Send a redux update to force an update.
window.store.dispatch(doUpdateUploadProgress({ guid: 'force--update' }));
break;
case TUS_REMOVED_UPLOADS:
// The other tab's store has removed this upload, so it's safe to do the
// same without affecting rehydration.
if (localStorageAvailable) {
const removedUploads = getRemovedUploads();
removedUploads.forEach((guid) => window.store.dispatch(doUpdateUploadRemove(guid)));
}
break;
}
}

View file

@ -30,10 +30,12 @@ export function makeUploadRequest(
delete params['remote_url'];
}
const { uploadUrl, guid, ...sdkParams } = params;
const jsonPayload = JSON.stringify({
jsonrpc: '2.0',
method: ENDPOINT_METHOD,
params,
params: sdkParams,
id: new Date().getTime(),
});
@ -47,18 +49,18 @@ export function makeUploadRequest(
xhr.responseType = 'json';
xhr.upload.onprogress = (e) => {
const percentage = ((e.loaded / e.total) * 100).toFixed(2);
window.store.dispatch(doUpdateUploadProgress({ params, progress: percentage }));
window.store.dispatch(doUpdateUploadProgress({ guid, progress: percentage }));
};
xhr.onload = () => {
window.store.dispatch(doUpdateUploadRemove(params));
window.store.dispatch(doUpdateUploadRemove(guid));
resolve(xhr);
};
xhr.onerror = () => {
window.store.dispatch(doUpdateUploadProgress({ params, status: 'error' }));
window.store.dispatch(doUpdateUploadProgress({ guid, status: 'error' }));
reject(new Error(__('There was a problem with your upload. Please try again.')));
};
xhr.onabort = () => {
window.store.dispatch(doUpdateUploadRemove(params));
window.store.dispatch(doUpdateUploadRemove(guid));
};
if (!isPreview) {

View file

@ -38,13 +38,12 @@ export function makeResumableUploadRequest(
reject(new Error('Publish: v2 does not support remote_url'));
}
const payloadParams = Object.assign({}, params);
delete payloadParams.uploadUrl; // cleanup
const { uploadUrl, guid, ...sdkParams } = params;
const jsonPayload = JSON.stringify({
jsonrpc: '2.0',
method: RESUMABLE_ENDPOINT_METHOD,
params: payloadParams,
params: sdkParams,
id: new Date().getTime(),
});
@ -70,7 +69,7 @@ export function makeResumableUploadRequest(
filetype: file instanceof File ? file.type : undefined,
},
onShouldRetry: (err, retryAttempt, options) => {
window.store.dispatch(doUpdateUploadProgress({ params, status: 'retry' }));
window.store.dispatch(doUpdateUploadProgress({ guid, status: 'retry' }));
const status = err.originalResponse ? err.originalResponse.getStatus() : 0;
return !inStatusCategory(status, 400);
},
@ -79,17 +78,17 @@ export function makeResumableUploadRequest(
const errMsg = typeof err === 'string' ? err : err.message;
if (status === STATUS_CONFLICT || status === STATUS_LOCKED || errMsg === 'file currently locked') {
window.store.dispatch(doUpdateUploadProgress({ params, status: 'conflict' }));
window.store.dispatch(doUpdateUploadProgress({ guid, status: 'conflict' }));
// prettier-ignore
reject(new Error(`${status}: concurrent upload detected. Uploading the same file from multiple tabs or windows is not allowed.`));
} else {
window.store.dispatch(doUpdateUploadProgress({ params, status: 'error' }));
window.store.dispatch(doUpdateUploadProgress({ guid, status: 'error' }));
reject(new Error(err));
}
},
onProgress: (bytesUploaded, bytesTotal) => {
const percentage = ((bytesUploaded / bytesTotal) * 100).toFixed(2);
window.store.dispatch(doUpdateUploadProgress({ params, progress: percentage }));
window.store.dispatch(doUpdateUploadProgress({ guid, progress: percentage }));
},
onSuccess: () => {
let retries = 1;
@ -102,7 +101,7 @@ export function makeResumableUploadRequest(
xhr.setRequestHeader(X_LBRY_AUTH_TOKEN, token);
xhr.responseType = 'json';
xhr.onload = () => {
window.store.dispatch(doUpdateUploadRemove(params));
window.store.dispatch(doUpdateUploadRemove(guid));
resolve(xhr);
};
xhr.onerror = () => {
@ -111,12 +110,12 @@ export function makeResumableUploadRequest(
analytics.error('notify: first attempt failed (status=0). Retrying after 10s...');
setTimeout(() => makeNotifyRequest(), 10000); // Auto-retry after 10s delay.
} else {
window.store.dispatch(doUpdateUploadProgress({ params, status: 'error' }));
window.store.dispatch(doUpdateUploadProgress({ guid, status: 'error' }));
reject(new Error(`There was a problem in the processing. Please retry. (${xhr.status})`));
}
};
xhr.onabort = () => {
window.store.dispatch(doUpdateUploadRemove(params));
window.store.dispatch(doUpdateUploadRemove(guid));
};
xhr.send(jsonPayload);

View file

@ -1,5 +1,6 @@
// @flow
import * as tus from 'tus-js-client';
import { v4 as uuid } from 'uuid';
import { makeUploadRequest } from './publish-v1';
import { makeResumableUploadRequest } from './publish-v2';
@ -34,6 +35,12 @@ export default function apiPublishCallViaWeb(
params.file_path = '__POST_FILE__';
}
// Add a random ID to serve as the redux key.
// If it already exists, then it is a resumed session.
if (!params.guid) {
params.guid = uuid();
}
const useV1 = remoteUrl || preview || !tus.isSupported;
// Note: both function signature (params) should match.