Skip to content

Commit

Permalink
@uppy/transloadit: fix unhandledPromiseRejection failures (#3197)
Browse files Browse the repository at this point in the history
* @uppy/transloadit: fix unhandledPromiseRejection failures

Fixes: #3189

* Add transloadit2 to end2end test matrix
  • Loading branch information
aduh95 committed Sep 16, 2021
1 parent c7e0a5a commit b364708
Show file tree
Hide file tree
Showing 7 changed files with 190 additions and 67 deletions.
91 changes: 38 additions & 53 deletions packages/@uppy/core/src/index.js
Expand Up @@ -563,9 +563,8 @@ class Uppy {
const { hasOwnProperty } = Object.prototype

const errors = []
const fileIDs = Object.keys(files)
for (let i = 0; i < fileIDs.length; i++) {
const file = this.getFile(fileIDs[i])
for (const fileID of Object.keys(files)) {
const file = this.getFile(fileID)
for (let i = 0; i < requiredMetaFields.length; i++) {
if (!hasOwnProperty.call(file.meta, requiredMetaFields[i]) || file.meta[requiredMetaFields[i]] === '') {
const err = new RestrictionError(`${this.i18n('missingRequiredMetaFieldOnFile', { fileName: file.name })}`)
Expand Down Expand Up @@ -1585,28 +1584,22 @@ class Uppy {
*
* @private
*/
#runUpload (uploadID) {
const uploadData = this.getState().currentUploads[uploadID]
const restoreStep = uploadData.step
async #runUpload (uploadID) {
let { currentUploads } = this.getState()
let currentUpload = currentUploads[uploadID]
const restoreStep = currentUpload.step || 0

const steps = [
...this.#preProcessors,
...this.#uploaders,
...this.#postProcessors,
]
let lastStep = Promise.resolve()
steps.forEach((fn, step) => {
// Skip this step if we are restoring and have already completed this step before.
if (step < restoreStep) {
return
}

lastStep = lastStep.then(() => {
const { currentUploads } = this.getState()
const currentUpload = currentUploads[uploadID]
try {
for (let step = restoreStep; step < steps.length; step++) {
if (!currentUpload) {
return
break
}
const fn = steps[step]

const updatedUpload = {
...currentUpload,
Expand All @@ -1622,25 +1615,20 @@ class Uppy {

// TODO give this the `updatedUpload` object as its only parameter maybe?
// Otherwise when more metadata may be added to the upload this would keep getting more parameters
return fn(updatedUpload.fileIDs, uploadID) // eslint-disable-line consistent-return
}).then(() => null)
})
await fn(updatedUpload.fileIDs, uploadID)

// Not returning the `catch`ed promise, because we still want to return a rejected
// promise from this method if the upload failed.
lastStep.catch((err) => {
// Update currentUpload value in case it was modified asynchronously.
currentUploads = this.getState().currentUploads
currentUpload = currentUploads[uploadID]
}
} catch (err) {
this.emit('error', err)
this.#removeUpload(uploadID)
})

return lastStep.then(() => {
// Set result data.
const { currentUploads } = this.getState()
const currentUpload = currentUploads[uploadID]
if (!currentUpload) {
return
}
throw err
}

// Set result data.
if (currentUpload) {
// Mark postprocessing step as complete if necessary; this addresses a case where we might get
// stuck in the postprocessing UI while the upload is fully complete.
// If the postprocessing steps do not do any work, they may not emit postprocessing events at
Expand All @@ -1661,30 +1649,27 @@ class Uppy {
const files = currentUpload.fileIDs.map((fileID) => this.getFile(fileID))
const successful = files.filter((file) => !file.error)
const failed = files.filter((file) => file.error)
this.addResultData(uploadID, { successful, failed, uploadID })
}).then(() => {
// Emit completion events.
// This is in a separate function so that the `currentUploads` variable
// always refers to the latest state. In the handler right above it refers
// to an outdated object without the `.result` property.
const { currentUploads } = this.getState()
if (!currentUploads[uploadID]) {
return
}
const currentUpload = currentUploads[uploadID]
const { result } = currentUpload
await this.addResultData(uploadID, { successful, failed, uploadID })

// Update currentUpload value in case it was modified asynchronously.
currentUploads = this.getState().currentUploads
currentUpload = currentUploads[uploadID]
}
// Emit completion events.
// This is in a separate function so that the `currentUploads` variable
// always refers to the latest state. In the handler right above it refers
// to an outdated object without the `.result` property.
let result
if (currentUpload) {
result = currentUpload.result
this.emit('complete', result)

this.#removeUpload(uploadID)

// eslint-disable-next-line consistent-return
return result
}).then((result) => {
if (result == null) {
this.log(`Not setting result for an upload that has been removed: ${uploadID}`)
}
return result
})
}
if (result == null) {
this.log(`Not setting result for an upload that has been removed: ${uploadID}`)
}
return result
}

/**
Expand Down
28 changes: 16 additions & 12 deletions packages/@uppy/transloadit/src/index.js
Expand Up @@ -14,6 +14,12 @@ function defaultGetAssemblyOptions (file, options) {
}
}

const sendErrorToConsole = originalErr => err => {
const error = new Error('Failed to send error to the client')
error.cause = err
console.error(error, originalErr)
}

const COMPANION = 'https://api2.transloadit.com/companion'
// Regex matching acceptable postMessage() origins for authentication feedback from companion.
const ALLOWED_COMPANION_PATTERN = /\.transloadit\.com$/
Expand Down Expand Up @@ -387,7 +393,7 @@ module.exports = class Transloadit extends BasePlugin {
#onCancelAll =() => {
const { uploadsAssemblies } = this.getPluginState()

const assemblyIDs = Object.values(uploadsAssemblies)
const assemblyIDs = Object.values(uploadsAssemblies).flat(1)

const cancelPromises = assemblyIDs.map((assemblyID) => {
const assembly = this.getAssembly(assemblyID)
Expand All @@ -405,10 +411,8 @@ module.exports = class Transloadit extends BasePlugin {
*
* @param {Function} setData
*/
#getPersistentData =(setData) => {
const state = this.getPluginState()
const { assemblies } = state
const { uploadsAssemblies } = state
#getPersistentData = (setData) => {
const { assemblies, uploadsAssemblies } = this.getPluginState()

setData({
[this.id]: {
Expand Down Expand Up @@ -473,11 +477,9 @@ module.exports = class Transloadit extends BasePlugin {
// Set up the assembly watchers again for all the ongoing uploads.
Object.keys(uploadsAssemblies).forEach((uploadID) => {
const assemblyIDs = uploadsAssemblies[uploadID]
const fileIDsInUpload = assemblyIDs.reduce((acc, assemblyID) => {
const fileIDsInAssembly = this.getAssemblyFiles(assemblyID).map((file) => file.id)
acc.push(...fileIDsInAssembly)
return acc
}, [])
const fileIDsInUpload = assemblyIDs.flatMap((assemblyID) => {
return this.getAssemblyFiles(assemblyID).map((file) => file.id)
})
this.#createAssemblyWatcher(assemblyIDs, fileIDsInUpload, uploadID)
})

Expand Down Expand Up @@ -704,15 +706,17 @@ module.exports = class Transloadit extends BasePlugin {
}
})
this.client.submitError(err)
// if we can't report the error that sucks
.catch(sendErrorToConsole(err))
}

#onTusError =(err) => {
if (err && /^tus: /.test(err.message)) {
const xhr = err.originalRequest ? err.originalRequest.getUnderlyingObject() : null
const url = xhr && xhr.responseURL ? xhr.responseURL : null
this.client.submitError(err, { url, type: 'TUS_ERROR' }).then(() => {
this.client.submitError(err, { url, type: 'TUS_ERROR' })
// if we can't report the error that sucks
})
.catch(sendErrorToConsole(err))
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/@uppy/transloadit/src/index.test.js
Expand Up @@ -33,10 +33,11 @@ describe('Transloadit', () => {
})

it('Does not leave lingering progress if getAssemblyOptions fails', () => {
const error = new Error('expected failure')
const uppy = new Core()
uppy.use(Transloadit, {
getAssemblyOptions () {
return Promise.reject(new Error('Failure!'))
return Promise.reject(error)
},
})

Expand All @@ -51,7 +52,7 @@ describe('Transloadit', () => {
}).catch((err) => {
const fileID = Object.keys(uppy.getState().files)[0]

expect(err.message).toBe('Failure!')
expect(err).toBe(error)
expect(uppy.getFile(fileID).progress.uploadStarted).toBe(null)
})
})
Expand Down
23 changes: 23 additions & 0 deletions test/endtoend/transloadit2/index.html
@@ -0,0 +1,23 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Uppy test page</title>
</head>
<body>
<style>
main {
max-width: 700px;
margin: auto;
}
</style>
<main>
<h2>Uppy Transloadit</h2>
<div id="uppy-transloadit"></div>
</main>

<link href="uppy.min.css" rel="stylesheet">
<script src="bundle.js"></script>
</body>
</html>
57 changes: 57 additions & 0 deletions test/endtoend/transloadit2/main.js
@@ -0,0 +1,57 @@
const Uppy = require('@uppy/core')
const Dashboard = require('@uppy/dashboard')
const Transloadit = require('@uppy/transloadit')

function initUppyTransloadit (transloaditKey) {
const uppyTransloadit = new Uppy({
id: 'uppyTransloadit',
debug: true,
autoProceed: true,
})

uppyTransloadit
.use(Dashboard, {
target: '#uppy-transloadit',
inline: true,
})
.use(Transloadit, {
service: 'https://api2-ap-southeast-1.transloadit.com',
params: {
steps: {
crop_thumbed: {
use: [':original'],
robot: '/image/resize',
height: 100,
resize_strategy: 'crop',
width: 100,
},
},
},
getAssemblyOptions () {
return {
params: {
auth: { key: transloaditKey },
template_id: 'uppyTransloadit',
},
}
},
waitForEncoding: true,
})

uppyTransloadit.on('transloadit:result', (stepName, result) => {
// use transloadit encoding result here.
console.log('Result here ====>', stepName, result)
console.log('Cropped image url is here ====>', result.url)

const img = new Image()
img.onload = function onload () {
const result = document.createElement('div')
result.setAttribute('id', 'uppy-result')
result.textContent = 'ok'
document.body.appendChild(result)
}
img.src = result.url
})
}

window.initUppyTransloadit = initUppyTransloadit
52 changes: 52 additions & 0 deletions test/endtoend/transloadit2/test.js
@@ -0,0 +1,52 @@
/* global browser, expect, capabilities, $ */
const path = require('path')
const fs = require('fs')
const { selectFakeFile, supportsChooseFile, ensureInputVisible } = require('../utils')

const testURL = 'http://localhost:4567/transloadit'

function setTransloaditKeyAndInit (transloaditKey) {
window.initUppyTransloadit(transloaditKey)
}

describe('Transloadit file processing', () => {
beforeEach(async () => {
await browser.url(testURL)
})

it('should upload a file to Transloadit and crop it', async function test () {
const transloaditKey = process.env.TRANSLOADIT_KEY
if (transloaditKey === undefined) {
console.log('skipping Transloadit integration test')
return this.skip()
}

const wrapper = await $('#uppy-transloadit')
await wrapper.waitForExist()

await browser.execute(setTransloaditKeyAndInit, transloaditKey)

const input = await $('#uppy-transloadit .uppy-Dashboard-input')
const result = await $('#uppy-result')

await input.waitForExist()
await browser.execute(ensureInputVisible, '#uppy-transloadit .uppy-Dashboard-input')

if (supportsChooseFile(capabilities)) {
await input.setValue(path.join(__dirname, '../../resources/image.jpg'))
} else {
const img = path.join(__dirname, '../../resources/image.jpg')
await browser.execute(
selectFakeFile,
'uppyTransloadit',
path.basename(img), // name
'image/jpeg', // type
fs.readFileSync(img, 'base64') // b64
)
// browser.execute(selectFakeFile, 'uppyTransloadit')
}
await result.waitForExist(25000)
const text = await result.getText()
expect(text).to.be.equal('ok')
})
})
1 change: 1 addition & 0 deletions test/endtoend/wdio.base.conf.js
Expand Up @@ -95,6 +95,7 @@ exports.config = {
{ mount: '/providers', path: './providers/dist' },
{ mount: '/thumbnails', path: './thumbnails/dist' },
{ mount: '/transloadit', path: './transloadit/dist' },
{ mount: '/transloadit2', path: './transloadit2/dist' },
{ mount: '/tus-drag-drop', path: './tus-drag-drop/dist' },
{ mount: '/typescript', path: './typescript/dist' },
{ mount: '/url-plugin', path: './url-plugin/dist' },
Expand Down

0 comments on commit b364708

Please sign in to comment.