Merge pull request #2053 from transloadit/provider-errors

companion: return  more accurate error status codes

packages/@uppy/companion/src/companion.js

@@ -18,6 +18,7 @@ const logger = require('./server/logger')
 const { STORAGE_PREFIX } = require('./server/Uploader')
 const middlewares = require('./server/middlewares')
 const { shortenToken } = require('./server/Uploader')
+const { ProviderApiError, ProviderAuthError } = require('./server/provider/error')
 
 const defaultOptions = {
   server: {
@@ -36,6 +37,9 @@ const defaultOptions = {
   debug: true
 }
 
+// make the errors available publicly for custom providers
+module.exports.errors = { ProviderApiError, ProviderAuthError }
+
 /**
  * Entry point into initializing the Companion app.
  *

packages/@uppy/companion/src/server/controllers/get.js

@@ -1,5 +1,6 @@
 const Uploader = require('../Uploader')
 const logger = require('../logger')
+const { errorToResponse } = require('../provider/error')
 
 function get (req, res, next) {
   const providerName = req.params.providerName
@@ -10,7 +11,11 @@ function get (req, res, next) {
   // get the file size before proceeding
   provider.size({ id, token, query: req.query }, (err, size) => {
     if (err) {
-      return err.isAuthError ? res.sendStatus(401) : next(err)
+      const errResp = errorToResponse(err)
+      if (errResp) {
+        return res.status(errResp.code).json({ message: errResp.message })
+      }
+      return next(err)
     }
 
     if (!size) {

packages/@uppy/companion/src/server/controllers/list.js

@@ -1,10 +1,16 @@
+const { errorToResponse } = require('../provider/error')
+
 function list ({ query, params, companion }, res, next) {
   const providerName = params.providerName
   const token = companion.providerTokens[providerName]
 
   companion.provider.list({ companion, token, directory: params.id, query }, (err, data) => {
     if (err) {
-      return err.isAuthError ? res.sendStatus(401) : next(err)
+      const errResp = errorToResponse(err)
+      if (errResp) {
+        return res.status(errResp.code).json({ message: errResp.message })
+      }
+      return next(err)
     }
     return res.json(data)
   })

packages/@uppy/companion/src/server/controllers/logout.js

@@ -1,4 +1,5 @@
 const tokenService = require('../helpers/jwt')
+const { errorToResponse } = require('../provider/error')
 
 /**
  *
@@ -17,6 +18,10 @@ function logout (req, res, next) {
   if (token) {
     req.companion.provider.logout({ token }, (err, data) => {
       if (err) {
+        const errResp = errorToResponse(err)
+        if (errResp) {
+          return res.status(errResp.code).json({ message: errResp.message })
+        }
         return next(err)
       }
 

packages/@uppy/companion/src/server/provider/drive/index.js

@@ -5,12 +5,15 @@ const request = require('request')
 const purest = require('purest')({ request })
 const logger = require('../../logger')
 const adapter = require('./adapter')
-const AuthError = require('../error')
+const { ProviderApiError, ProviderAuthError } = require('../error')
 const DRIVE_FILE_FIELDS = 'kind,id,name,mimeType,ownedByMe,permissions(role,emailAddress),size,modifiedTime,iconLink,thumbnailLink,teamDriveId'
 const DRIVE_FILES_FIELDS = `kind,nextPageToken,incompleteSearch,files(${DRIVE_FILE_FIELDS})`
 // using wildcard to get all 'drive' fields because specifying fields seems no to work for the /drives endpoint
 const SHARED_DRIVE_FIELDS = '*'
 
+/**
+ * Adapter for API https://developers.google.com/drive/api/v3/
+ */
 class Drive extends Provider {
   constructor (options) {
     super(options)
@@ -184,8 +187,9 @@ class Drive extends Provider {
 
   _error (err, resp) {
     if (resp) {
-      const errMsg = `request to ${this.authProvider} returned ${resp.statusCode}`
-      return resp.statusCode === 401 ? new AuthError() : new Error(errMsg)
+      const fallbackMessage = `request to ${this.authProvider} returned ${resp.statusCode}`
+      const errMsg = resp.body.error ? resp.body.error.message : fallbackMessage
+      return resp.statusCode === 401 ? new ProviderAuthError() : new ProviderApiError(errMsg, resp.statusCode)
     }
     return err
   }

packages/@uppy/companion/src/server/provider/dropbox/index.js

@@ -4,7 +4,7 @@ const request = require('request')
 const purest = require('purest')({ request })
 const logger = require('../../logger')
 const adapter = require('./adapter')
-const AuthError = require('../error')
+const { ProviderApiError, ProviderAuthError } = require('../error')
 
 // From https://www.dropbox.com/developers/reference/json-encoding:
 //
@@ -19,6 +19,9 @@ function httpHeaderSafeJson (v) {
   )
 }
 
+/**
+ * Adapter for API https://www.dropbox.com/developers/documentation/http/documentation
+ */
 class DropBox extends Provider {
   constructor (options) {
     super(options)
@@ -202,8 +205,9 @@ class DropBox extends Provider {
 
   _error (err, resp) {
     if (resp) {
-      const errMsg = `request to ${this.authProvider} returned ${resp.statusCode}`
-      return resp.statusCode === 401 ? new AuthError() : new Error(errMsg)
+      const fallbackMessage = `request to ${this.authProvider} returned ${resp.statusCode}`
+      const errMsg = resp.body.error_summary ? resp.body.error_summary : fallbackMessage
+      return resp.statusCode === 401 ? new ProviderAuthError() : new ProviderApiError(errMsg, resp.statusCode)
     }
 
     return err

packages/@uppy/companion/src/server/provider/error.js

@@ -1,13 +1,52 @@
+/**
+ * ProviderApiError is error returned when an adapter encounters
+ * an http error while communication with its corresponding provider
+ */
+class ProviderApiError extends Error {
+  /**
+   * @param {string} message error message
+   * @param {number} statusCode the http status code from the provider api
+   */
+  constructor (message, statusCode) {
+    super(message)
+    this.name = 'ProviderApiError'
+    this.statusCode = statusCode
+    this.isAuthError = false
+  }
+}
+
 /**
  * AuthError is error returned when an adapter encounters
  * an authorization error while communication with its corresponding provider
  */
-class AuthError extends Error {
+class ProviderAuthError extends ProviderApiError {
   constructor () {
-    super('invalid access token detected by Provider')
+    super('invalid access token detected by Provider', 401)
     this.name = 'AuthError'
     this.isAuthError = true
   }
 }
 
-module.exports = AuthError
+/**
+ * Convert an error instance to an http response if possible
+ * @param {Error | ProviderApiError} err the error instance to convert to an http json response
+ */
+function errorToResponse (err) {
+  if (err instanceof ProviderAuthError && err.isAuthError) {
+    return { code: 401, message: err.message }
+  }
+
+  if (err instanceof ProviderApiError) {
+    if (err.statusCode >= 500) {
+      // bad gateway i.e the provider APIs gateway
+      return { code: 502, message: err.message }
+    }
+
+    if (err.statusCode >= 400) {
+      // 424 Failed Dependency
+      return { code: 424, message: err.message }
+    }
+  }
+}
+
+module.exports = { ProviderAuthError, ProviderApiError, errorToResponse }

packages/@uppy/companion/src/server/provider/facebook/index.js

@@ -5,8 +5,11 @@ const purest = require('purest')({ request })
 const utils = require('../../helpers/utils')
 const logger = require('../../logger')
 const adapter = require('./adapter')
-const AuthError = require('../error')
+const { ProviderApiError, ProviderAuthError } = require('../error')
 
+/**
+ * Adapter for API https://developers.facebook.com/docs/graph-api/using-graph-api/
+ */
 class Facebook extends Provider {
   constructor (options) {
     super(options)
@@ -153,11 +156,12 @@ class Facebook extends Provider {
     if (resp) {
       if (resp.body && resp.body.error.code === 190) {
         // Invalid OAuth 2.0 Access Token
-        return new AuthError()
+        return new ProviderAuthError()
       }
 
-      const msg = resp.body && resp.body.error ? resp.body.error.message : ''
-      return new Error(`request to ${this.authProvider} returned status: ${resp.statusCode}, message: ${msg}`)
+      const fallbackMessage = `request to ${this.authProvider} returned ${resp.statusCode}`
+      const msg = resp.body && resp.body.error ? resp.body.error.message : fallbackMessage
+      return new ProviderApiError(msg, resp.statusCode)
     }
 
     return err

packages/@uppy/companion/src/server/provider/instagram/graph/index.js

@@ -5,8 +5,11 @@ const purest = require('purest')({ request })
 const utils = require('../../../helpers/utils')
 const logger = require('../../../logger')
 const adapter = require('./adapter')
-const AuthError = require('../../error')
+const { ProviderApiError, ProviderAuthError } = require('../../error')
 
+/**
+ * Adapter for API https://developers.facebook.com/docs/instagram-api/overview
+ */
 class Instagram extends Provider {
   constructor (options) {
     super(options)
@@ -140,11 +143,12 @@ class Instagram extends Provider {
     if (resp) {
       if (resp.body && resp.body.error.code === 190) {
         // Invalid OAuth 2.0 Access Token
-        return new AuthError()
+        return new ProviderAuthError()
       }
 
-      const msg = resp.body && resp.body.error ? resp.body.error.message : ''
-      return new Error(`request to ${this.authProvider} returned status: ${resp.statusCode}, message: ${msg}`)
+      const fallbackMessage = `request to ${this.authProvider} returned ${resp.statusCode}`
+      const msg = resp.body && resp.body.error ? resp.body.error.message : fallbackMessage
+      return new ProviderApiError(msg, resp.statusCode)
     }
 
     return err

packages/@uppy/companion/src/server/provider/instagram/index.js

@@ -5,8 +5,11 @@ const purest = require('purest')({ request })
 const utils = require('../../helpers/utils')
 const logger = require('../../logger')
 const adapter = require('./adapter')
-const AuthError = require('../error')
+const { ProviderApiError, ProviderAuthError } = require('../error')
 
+/**
+ * Adapter for API https://www.instagram.com/developer/endpoints/
+ */
 class Instagram extends Provider {
   constructor (options) {
     super(options)
@@ -139,10 +142,11 @@ class Instagram extends Provider {
   _error (err, resp) {
     if (resp) {
       if (resp.statusCode === 400 && resp.body && resp.body.meta.error_type === 'OAuthAccessTokenException') {
-        return new AuthError()
+        return new ProviderAuthError()
       }
 
-      return new Error(`request to ${this.authProvider} returned ${resp.statusCode}`)
+      const msg = `request to ${this.authProvider} returned ${resp.statusCode}`
+      return new ProviderApiError(msg, resp.statusCode)
     }
 
     return err

packages/@uppy/companion/src/server/provider/onedrive/index.js

@@ -4,8 +4,11 @@ const request = require('request')
 const purest = require('purest')({ request })
 const logger = require('../../logger')
 const adapter = require('./adapter')
-const AuthError = require('../error')
+const { ProviderApiError, ProviderAuthError } = require('../error')
 
+/**
+ * Adapter for API https://docs.microsoft.com/en-us/onedrive/developer/rest-api/
+ */
 class OneDrive extends Provider {
   constructor (options) {
     super(options)
@@ -126,8 +129,9 @@ class OneDrive extends Provider {
 
   _error (err, resp) {
     if (resp) {
-      const errMsg = `request to ${this.authProvider} returned ${resp.statusCode}`
-      return resp.statusCode === 401 ? new AuthError() : new Error(errMsg)
+      const fallbackMsg = `request to ${this.authProvider} returned ${resp.statusCode}`
+      const errMsg = resp.body.error ? resp.body.error.message : fallbackMsg
+      return resp.statusCode === 401 ? new ProviderAuthError() : new ProviderApiError(errMsg, resp.statusCode)
     }
 
     return err