Improve errors (#211)

* Improve errors Renamed `TransloaditError` to `ApiError`. Differences between `TransloaditError` and `ApiError`: - Moved `TransloaditError.response.body` to `ApiError.response` - Removed `TransloaditError.assemblyId` (can now be found in `ApiError.response.assembly_id` - Removed `TransloaditError.transloaditErrorCode` (can now be found in `ApiError.response.error` - `ApiError` does not inherit from `got.HTTPError`, but `ApiError.cause` will be the `got.HTTPError` instance that caused this error (except for when Tranloadit API responds with HTTP 200 and `error` prop set in JSON response, in which case cause will be `undefined`). Note that (just like before) when the Transloadit API responds with an error we will always throw a `ApiError` - In all other cases (like request timeout, connection error, TypeError etc.), we don't wrap the error in `ApiError`. Also improved error stack traces, added a unit test in `mock-http.test.ts` that verifies the stack trace. * Improve errors alternative implementation (#212) * make alternative implementation ...of #211 where `ApiError` has all the API response properties directly on it instaed of inside a `response` object - `ApiError.response.error` -> `ApiError.code` - `ApiError.response.message` -> `ApiError.rawMessage` - `ApiError.response.assembly_id` -> `ApiError.assemblyId` - `ApiError.response.assembly_ssl_url` -> `ApiError.assemblySslUrl` * fix formatting * Update README.md Co-authored-by: Remco Haszing <[email protected]> * remove stack hack #211 (comment) * improve assertion * fix typo * fix formatting --------- Co-authored-by: Remco Haszing <[email protected]> --------- Co-authored-by: Remco Haszing <[email protected]>
transloadit · Dec 19, 2024 · 94356cb · 94356cb
1 parent 98a6419
commit 94356cb
Show file tree

Hide file tree

Showing 7 changed files with 165 additions and 188 deletions.
diff --git a/README.md b/README.md
@@ -56,45 +56,41 @@ const transloadit = new Transloadit({
   authSecret: 'YOUR_TRANSLOADIT_SECRET',
 })
 
-;(async () => {
-  try {
-    const options = {
-      files: {
-        file1: '/PATH/TO/FILE.jpg',
-      },
-      params: {
-        steps: {
-          // You can have many Steps. In this case we will just resize any inputs (:original)
-          resize: {
-            use: ':original',
-            robot: '/image/resize',
-            result: true,
-            width: 75,
-            height: 75,
-          },
+try {
+  const options = {
+    files: {
+      file1: '/PATH/TO/FILE.jpg',
+    },
+    params: {
+      steps: {
+        // You can have many Steps. In this case we will just resize any inputs (:original)
+        resize: {
+          use: ':original',
+          robot: '/image/resize',
+          result: true,
+          width: 75,
+          height: 75,
         },
-        // OR if you already created a template, you can use it instead of "steps":
-        // template_id: 'YOUR_TEMPLATE_ID',
       },
-      waitForCompletion: true, // Wait for the Assembly (job) to finish executing before returning
-    }
-
-    const status = await transloadit.createAssembly(options)
-
-    if (status.results.resize) {
-      console.log('✅ Success - Your resized image:', status.results.resize[0].ssl_url)
-    } else {
-      console.log(
-        "❌ The Assembly didn't produce any output. Make sure you used a valid image file"
-      )
-    }
-  } catch (err) {
-    console.error('❌ Unable to process Assembly.', err)
-    if (err.cause?.assembly_id) {
-      console.error(`💡 More info: https://transloadit.com/assemblies/${err.cause?.assembly_id}`)
-    }
+      // OR if you already created a template, you can use it instead of "steps":
+      // template_id: 'YOUR_TEMPLATE_ID',
+    },
+    waitForCompletion: true, // Wait for the Assembly (job) to finish executing before returning
+  }
+
+  const status = await transloadit.createAssembly(options)
+
+  if (status.results.resize) {
+    console.log('✅ Success - Your resized image:', status.results.resize[0].ssl_url)
+  } else {
+    console.log("❌ The Assembly didn't produce any output. Make sure you used a valid image file")
   }
-})()
+} catch (err) {
+  console.error('❌ Unable to process Assembly.', err)
+  if (err instanceof ApiError && err.assemblyId) {
+    console.error(`💡 More info: https://transloadit.com/assemblies/${err.assemblyId}`)
+  }
+}
 ```
 
 You can find [details about your executed Assemblies here](https://transloadit.com/assemblies).
@@ -419,31 +415,34 @@ const url = client.getSignedSmartCDNUrl({
 
 ### Errors
 
-Errors from Node.js will be passed on and we use [GOT](https://github.com/sindresorhus/got) for HTTP requests and errors from there will also be passed on. When the HTTP response code is not 200, the error will be an `HTTPError`, which is a [got.HTTPError](https://github.com/sindresorhus/got#errors)) with some additional properties:
+Any errors originating from Node.js will be passed on and we use [GOT](https://github.com/sindresorhus/got) v11 for HTTP requests. [Errors from `got`](https://github.com/sindresorhus/got/tree/v11.8.6?tab=readme-ov-file#errors) will also be passed on, _except_ the `got.HTTPError` which will be replaced with a `transloadit.ApiError`, which will have its `cause` property set to the instance of the original `got.HTTPError`. `transloadit.ApiError` has these properties:
 
-- **(deprecated: use `cause` instead)** `HTTPError.response?.body` the JSON object returned by the server along with the error response (**note**: `HTTPError.response` will be `undefined` for non-server errors)
-- **(deprecated)** `HTTPError.transloaditErrorCode` alias for `HTTPError.cause?.error` ([View all error codes](https://transloadit.com/docs/api/response-codes/#error-codes))
-- `HTTPError.assemblyId` (alias for `HTTPError.response.body.assembly_id`, if the request regards an [Assembly](https://transloadit.com/docs/api/assemblies-assembly-id-get/))
+- `code` (`string`) - [The Transloadit API error code](https://transloadit.com/docs/api/response-codes/#error-codes).
+- `rawMessage` (`string`) - A textual representation of the Transloadit API error.
+- `assemblyId`: (`string`) - If the request is related to an assembly, this will be the ID of the assembly.
+- `assemblySslUrl` (`string`) - If the request is related to an assembly, this will be the SSL URL to the assembly .
 
 To identify errors you can either check its props or use `instanceof`, e.g.:
 
 ```js
-catch (err) {
-  if (err instanceof TimeoutError) {
+try {
+  await transloadit.createAssembly(options)
+} catch (err) {
+  if (err instanceof got.TimeoutError) {
     return console.error('The request timed out', err)
   }
   if (err.code === 'ENOENT') {
     return console.error('Cannot open file', err)
   }
-  if (err.cause?.error === 'ASSEMBLY_INVALID_STEPS') {
+  if (err instanceof ApiError && err.code === 'ASSEMBLY_INVALID_STEPS') {
     return console.error('Invalid Assembly Steps', err)
   }
 }
 ```
 
-**Note:** Assemblies that have an error status (`assembly.error`) will only result in an error thrown from `createAssembly` and `replayAssembly`. For other Assembly methods, no errors will be thrown, but any error can be found in the response's `error` property
+**Note:** Assemblies that have an error status (`assembly.error`) will only result in an error being thrown from `createAssembly` and `replayAssembly`. For other Assembly methods, no errors will be thrown, but any error can be found in the response's `error` property (also `ApiError.code`).
 
-- [More information on Transloadit errors (`cause.error`)](https://transloadit.com/docs/api/response-codes/#error-codes)
+- [More information on Transloadit errors (`ApiError.code`)](https://transloadit.com/docs/api/response-codes/#error-codes)
 - [More information on request errors](https://github.com/sindresorhus/got#errors)
 
 ### Rate limiting & auto retry

diff --git a/examples/retry.js b/examples/retry.js
@@ -9,7 +9,7 @@
 //   yarn prepack
 //
 const pRetry = require('p-retry')
-const { Transloadit, TransloaditError } = require('transloadit')
+const { Transloadit, ApiError } = require('transloadit')
 
 const transloadit = new Transloadit({
   authKey: /** @type {string} */ (process.env.TRANSLOADIT_KEY),
@@ -22,7 +22,7 @@ async function run() {
     const { items } = await transloadit.listTemplates({ sort: 'created', order: 'asc' })
     return items
   } catch (err) {
-    if (err instanceof TransloaditError && err.cause?.error === 'INVALID_SIGNATURE') {
+    if (err instanceof ApiError && err.code === 'INVALID_SIGNATURE') {
       // This is an unrecoverable error, abort retry
       throw new pRetry.AbortError('INVALID_SIGNATURE')
     }

diff --git a/src/ApiError.ts b/src/ApiError.ts
@@ -0,0 +1,40 @@
+import { HTTPError } from 'got'
+
+export interface TransloaditErrorResponseBody {
+  error?: string
+  message?: string
+  assembly_ssl_url?: string
+  assembly_id?: string
+}
+
+export class ApiError extends Error {
+  override name = 'ApiError'
+
+  // there might not be an error code (or message) if the server didn't respond with any JSON response at all
+  // e.g. if there was a 500 in the HTTP reverse proxy
+  code?: string
+  rawMessage?: string
+  assemblySslUrl?: string
+  assemblyId?: string
+
+  override cause?: HTTPError | undefined
+
+  constructor(params: { cause?: HTTPError; body: TransloaditErrorResponseBody | undefined }) {
+    const { cause, body = {} } = params
+
+    const parts = ['API error']
+    if (cause?.response.statusCode) parts.push(`(HTTP ${cause.response.statusCode})`)
+    if (body.error) parts.push(`${body.error}:`)
+    if (body.message) parts.push(body.message)
+    if (body.assembly_ssl_url) parts.push(body.assembly_ssl_url)
+
+    const message = parts.join(' ')
+
+    super(message)
+    this.rawMessage = body.message
+    this.assemblyId = body.assembly_id
+    this.assemblySslUrl = body.assembly_ssl_url
+    this.code = body.error
+    this.cause = cause
+  }
+}
diff --git a/src/Transloadit.ts b/src/Transloadit.ts
@@ -1,5 +1,5 @@
 import { createHmac, randomUUID } from 'crypto'
-import got, { RequiredRetryOptions, Headers, OptionsOfJSONResponseBody, HTTPError } from 'got'
+import got, { RequiredRetryOptions, Headers, OptionsOfJSONResponseBody } from 'got'
 import FormData from 'form-data'
 import { constants, createReadStream } from 'fs'
 import { access } from 'fs/promises'
@@ -11,13 +11,13 @@ import pMap from 'p-map'
 import { InconsistentResponseError } from './InconsistentResponseError'
 import { PaginationStream } from './PaginationStream'
 import { PollingTimeoutError } from './PollingTimeoutError'
-import { TransloaditResponseBody, TransloaditError } from './TransloaditError'
+import { TransloaditErrorResponseBody, ApiError } from './ApiError'
 import { version } from '../package.json'
 import { sendTusRequest, Stream } from './tus'
 
 import type { Readable } from 'stream'
 
-// See https://github.com/sindresorhus/got#errors
+// See https://github.com/sindresorhus/got/tree/v11.8.6?tab=readme-ov-file#errors
 // Expose relevant errors
 export {
   RequestError,
@@ -28,7 +28,7 @@ export {
   MaxRedirectsError,
   TimeoutError,
 } from 'got'
-export { InconsistentResponseError, TransloaditError }
+export { InconsistentResponseError, ApiError }
 
 const log = debug('transloadit')
 const logWarn = debug('transloadit:warn')
@@ -47,60 +47,6 @@ interface CreateAssemblyPromise extends Promise<Assembly> {
   assemblyId: string
 }
 
-function getTransloaditErrorPropsFromBody(err: Error, body: TransloaditResponseBody) {
-  let newMessage = err.message
-  let newStack = err.stack
-
-  // Provide a more useful message if there is one
-  if (body?.message && body?.error) newMessage += ` ${body.error}: ${body.message}`
-  else if (body?.error) newMessage += ` ${body.error}`
-
-  if (body?.assembly_ssl_url) newMessage += ` - ${body.assembly_ssl_url}`
-
-  if (typeof err.stack === 'string') {
-    const indexOfMessageEnd = err.stack.indexOf(err.message) + err.message.length
-    const stacktrace = err.stack.slice(indexOfMessageEnd)
-    newStack = `${newMessage}${stacktrace}`
-  }
-
-  return {
-    message: newMessage,
-    ...(newStack != null && { stack: newStack }),
-    ...(body?.assembly_id && { assemblyId: body.assembly_id }),
-    ...(body?.error && { transloaditErrorCode: body.error }),
-  }
-}
-
-function decorateTransloaditError(err: HTTPError, body: TransloaditResponseBody): TransloaditError {
-  // todo improve this
-  const transloaditErr = err as HTTPError & TransloaditError
-  /* eslint-disable no-param-reassign */
-  if (body) transloaditErr.cause = body
-  const props = getTransloaditErrorPropsFromBody(err, body)
-  transloaditErr.message = props.message
-  if (props.stack != null) transloaditErr.stack = props.stack
-  if (props.assemblyId) transloaditErr.assemblyId = props.assemblyId
-  if (props.transloaditErrorCode) transloaditErr.transloaditErrorCode = props.transloaditErrorCode
-  /* eslint-enable no-param-reassign */
-
-  return transloaditErr
-}
-
-function makeTransloaditError(err: Error, body: TransloaditResponseBody): TransloaditError {
-  const transloaditErr = new TransloaditError(err.message, body)
-  // todo improve this
-  /* eslint-disable no-param-reassign */
-  if (body) transloaditErr.cause = body
-  const props = getTransloaditErrorPropsFromBody(err, body)
-  transloaditErr.message = props.message
-  if (props.stack != null) transloaditErr.stack = props.stack
-  if (props.assemblyId) transloaditErr.assemblyId = props.assemblyId
-  if (props.transloaditErrorCode) transloaditErr.transloaditErrorCode = props.transloaditErrorCode
-  /* eslint-enable no-param-reassign */
-
-  return transloaditErr
-}
-
 // Not sure if this is still a problem with the API, but throw a special error type so the user can retry if needed
 function checkAssemblyUrls(result: Assembly) {
   if (result.assembly_url == null || result.assembly_ssl_url == null) {
@@ -114,14 +60,14 @@ function getHrTimeMs(): number {
 
 function checkResult<T>(result: T | { error: string }): asserts result is T {
   // In case server returned a successful HTTP status code, but an `error` in the JSON object
-  // This happens sometimes when createAssembly with an invalid file (IMPORT_FILE_ERROR)
+  // This happens sometimes, for example when createAssembly with an invalid file (IMPORT_FILE_ERROR)
   if (
     typeof result === 'object' &&
     result !== null &&
     'error' in result &&
     typeof result.error === 'string'
   ) {
-    throw makeTransloaditError(new Error('Error in response'), result)
+    throw new ApiError({ body: result }) // in this case there is no `cause` because we don't have an HTTPError
   }
 }
 
@@ -814,7 +760,10 @@ export class Transloadit {
             retryCount < this._maxRetries
           )
         ) {
-          throw decorateTransloaditError(err, body as TransloaditResponseBody) // todo improve
+          throw new ApiError({
+            cause: err,
+            body: body as TransloaditErrorResponseBody,
+          }) // todo don't assert type
         }
 
         const { retryIn: retryInSec } = body.info

diff --git a/src/TransloaditError.ts b/src/TransloaditError.ts
diff --git a/test/integration/live-api.test.ts b/test/integration/live-api.test.ts
@@ -398,11 +398,7 @@ describe('API integration', { timeout: 60000 }, () => {
       const promise = createAssembly(client, opts)
       await promise.catch((err) => {
         expect(err).toMatchObject({
-          transloaditErrorCode: 'INVALID_INPUT_ERROR',
-          cause: expect.objectContaining({
-            error: 'INVALID_INPUT_ERROR',
-            assembly_id: expect.any(String),
-          }),
+          code: 'INVALID_INPUT_ERROR',
           assemblyId: expect.any(String),
         })
       })
@@ -731,10 +727,7 @@ describe('API integration', { timeout: 60000 }, () => {
       expect(ok).toBe('TEMPLATE_DELETED')
       await expect(client.getTemplate(templId!)).rejects.toThrow(
         expect.objectContaining({
-          transloaditErrorCode: 'TEMPLATE_NOT_FOUND',
-          cause: expect.objectContaining({
-            error: 'TEMPLATE_NOT_FOUND',
-          }),
+          code: 'TEMPLATE_NOT_FOUND',
         })
       )
     })
@@ -805,10 +798,7 @@ describe('API integration', { timeout: 60000 }, () => {
       expect(ok).toBe('TEMPLATE_CREDENTIALS_DELETED')
       await expect(client.getTemplateCredential(credId!)).rejects.toThrow(
         expect.objectContaining({
-          transloaditErrorCode: 'TEMPLATE_CREDENTIALS_NOT_READ',
-          cause: expect.objectContaining({
-            error: 'TEMPLATE_CREDENTIALS_NOT_READ',
-          }),
+          code: 'TEMPLATE_CREDENTIALS_NOT_READ',
         })
       )
     })