From 0e0e9e1c051338a8ef2fadc5e32a392b830be29d Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Thu, 12 Dec 2024 18:22:35 +0800 Subject: [PATCH] improve TransloaditError (#210) - add `cause` to TranslodaitError which is the response body - deprecate `transloaditErrorCode` (use `cause.error`) - deprecate `response?.body` (use `cause`) - deprecate `assemblyId` (use `cause.assembly_id`) - make sure errors thrown when status 200 but with `body.error` are also TransloaditError --- README.md | 12 +++---- examples/retry.js | 2 +- src/Transloadit.ts | 58 ++++++++++++++++++++++--------- src/TransloaditError.ts | 28 +++++++++++++-- test/integration/live-api.test.ts | 18 ++++++++-- test/unit/mock-http.test.ts | 17 ++++++++- 6 files changed, 107 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index e1af857..509c977 100644 --- a/README.md +++ b/README.md @@ -90,8 +90,8 @@ const transloadit = new Transloadit({ } } catch (err) { console.error('❌ Unable to process Assembly.', err) - if (err.assemblyId) { - console.error(`💡 More info: https://transloadit.com/assemblies/${err.assemblyId}`) + if (err.cause?.assembly_id) { + console.error(`💡 More info: https://transloadit.com/assemblies/${err.cause?.assembly_id}`) } } })() @@ -421,8 +421,8 @@ const url = client.getSignedSmartCDNUrl({ 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: -- `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) -- `HTTPError.transloaditErrorCode` alias for `HTTPError.response.body.error` ([View all error codes](https://transloadit.com/docs/api/response-codes/#error-codes)) +- **(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/)) To identify errors you can either check its props or use `instanceof`, e.g.: @@ -435,7 +435,7 @@ catch (err) { if (err.code === 'ENOENT') { return console.error('Cannot open file', err) } - if (err.transloaditErrorCode === 'ASSEMBLY_INVALID_STEPS') { + if (err.cause?.error === 'ASSEMBLY_INVALID_STEPS') { return console.error('Invalid Assembly Steps', err) } } @@ -443,7 +443,7 @@ catch (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 -- [More information on Transloadit errors (`transloaditErrorCode`)](https://transloadit.com/docs/api/response-codes/#error-codes) +- [More information on Transloadit errors (`cause.error`)](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 index d967c82..98cc6fe 100644 --- a/examples/retry.js +++ b/examples/retry.js @@ -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.transloaditErrorCode === 'INVALID_SIGNATURE') { + if (err instanceof TransloaditError && err.cause?.error === 'INVALID_SIGNATURE') { // This is an unrecoverable error, abort retry throw new pRetry.AbortError('INVALID_SIGNATURE') } diff --git a/src/Transloadit.ts b/src/Transloadit.ts index b686188..4ca75f2 100644 --- a/src/Transloadit.ts +++ b/src/Transloadit.ts @@ -1,5 +1,5 @@ import { createHmac, randomUUID } from 'crypto' -import got, { RequiredRetryOptions, Headers, OptionsOfJSONResponseBody } from 'got' +import got, { RequiredRetryOptions, Headers, OptionsOfJSONResponseBody, HTTPError } from 'got' import FormData from 'form-data' import { constants, createReadStream } from 'fs' import { access } from 'fs/promises' @@ -11,7 +11,7 @@ import pMap from 'p-map' import { InconsistentResponseError } from './InconsistentResponseError' import { PaginationStream } from './PaginationStream' import { PollingTimeoutError } from './PollingTimeoutError' -import { TransloaditError } from './TransloaditError' +import { TransloaditResponseBody, TransloaditError } from './TransloaditError' import { version } from '../package.json' import { sendTusRequest, Stream } from './tus' @@ -47,17 +47,15 @@ interface CreateAssemblyPromise extends Promise { assemblyId: string } -function decorateHttpError(err: TransloaditError, body: any): TransloaditError { - if (!body) return err - +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?.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 (body?.assembly_ssl_url) newMessage += ` - ${body.assembly_ssl_url}` if (typeof err.stack === 'string') { const indexOfMessageEnd = err.stack.indexOf(err.message) + err.message.length @@ -65,14 +63,42 @@ function decorateHttpError(err: TransloaditError, body: any): TransloaditError { 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 */ - err.message = newMessage - if (newStack != null) err.stack = newStack - if (body.assembly_id) err.assemblyId = body.assembly_id - if (body.error) err.transloaditErrorCode = body.error + 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 err + 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 @@ -95,7 +121,7 @@ function checkResult(result: T | { error: string }): asserts result is T { 'error' in result && typeof result.error === 'string' ) { - throw decorateHttpError(new TransloaditError('Error in response', result), result) + throw makeTransloaditError(new Error('Error in response'), result) } } @@ -738,7 +764,7 @@ export class Transloadit { log('Sending request', method, url) - // Cannot use got.retry because we are using FormData which is a stream and can only be used once + // todo use got.retry instead because we are no longer using FormData (which is a stream and can only be used once) // https://github.com/sindresorhus/got/issues/1282 for (let retryCount = 0; ; retryCount++) { let form @@ -788,7 +814,7 @@ export class Transloadit { retryCount < this._maxRetries ) ) { - throw decorateHttpError(err, body) + throw decorateTransloaditError(err, body as TransloaditResponseBody) // todo improve } const { retryIn: retryInSec } = body.info diff --git a/src/TransloaditError.ts b/src/TransloaditError.ts index a67897a..6f5dde8 100644 --- a/src/TransloaditError.ts +++ b/src/TransloaditError.ts @@ -1,10 +1,34 @@ +export type TransloaditResponseBody = + | { + error?: string + message?: string + http_code?: string + assembly_ssl_url?: string + assembly_id?: string + } + | undefined + export class TransloaditError extends Error { override name = 'TransloaditError' - response: { body: unknown } + + /** + * @deprecated use `cause` instead. + */ + response: { body: TransloaditResponseBody } + + /** + * @deprecated use `cause.assembly_id` instead. + */ assemblyId?: string + + /** + * @deprecated use `cause?.error` instead. + */ transloaditErrorCode?: string - constructor(message: string, body: unknown) { + override cause?: TransloaditResponseBody + + constructor(message: string, body: TransloaditResponseBody) { super(message) this.response = { body } } diff --git a/test/integration/live-api.test.ts b/test/integration/live-api.test.ts index 4aa95b7..4d36f25 100644 --- a/test/integration/live-api.test.ts +++ b/test/integration/live-api.test.ts @@ -399,6 +399,10 @@ describe('API integration', { timeout: 60000 }, () => { await promise.catch((err) => { expect(err).toMatchObject({ transloaditErrorCode: 'INVALID_INPUT_ERROR', + cause: expect.objectContaining({ + error: 'INVALID_INPUT_ERROR', + assembly_id: expect.any(String), + }), assemblyId: expect.any(String), }) }) @@ -726,7 +730,12 @@ describe('API integration', { timeout: 60000 }, () => { const { ok } = template expect(ok).toBe('TEMPLATE_DELETED') await expect(client.getTemplate(templId!)).rejects.toThrow( - expect.objectContaining({ transloaditErrorCode: 'TEMPLATE_NOT_FOUND' }) + expect.objectContaining({ + transloaditErrorCode: 'TEMPLATE_NOT_FOUND', + cause: expect.objectContaining({ + error: 'TEMPLATE_NOT_FOUND', + }), + }) ) }) }) @@ -795,7 +804,12 @@ describe('API integration', { timeout: 60000 }, () => { const { ok } = credential expect(ok).toBe('TEMPLATE_CREDENTIALS_DELETED') await expect(client.getTemplateCredential(credId!)).rejects.toThrow( - expect.objectContaining({ transloaditErrorCode: 'TEMPLATE_CREDENTIALS_NOT_READ' }) + expect.objectContaining({ + transloaditErrorCode: 'TEMPLATE_CREDENTIALS_NOT_READ', + cause: expect.objectContaining({ + error: 'TEMPLATE_CREDENTIALS_NOT_READ', + }), + }) ) }) }) diff --git a/test/unit/mock-http.test.ts b/test/unit/mock-http.test.ts index 4946d4a..8320354 100644 --- a/test/unit/mock-http.test.ts +++ b/test/unit/mock-http.test.ts @@ -97,6 +97,9 @@ describe('Mocked API tests', () => { await expect(client.createAssembly()).rejects.toThrow( expect.objectContaining({ transloaditErrorCode: 'INVALID_FILE_META_DATA', + cause: { + error: 'INVALID_FILE_META_DATA', + }, message: 'Response code 400 (Bad Request) INVALID_FILE_META_DATA', }) ) @@ -119,6 +122,7 @@ describe('Mocked API tests', () => { response: expect.objectContaining({ body: expect.objectContaining({ assembly_id: '123' }), }), + cause: expect.objectContaining({ assembly_id: '123' }), }) ) }) @@ -152,6 +156,9 @@ describe('Mocked API tests', () => { await expect(client.createAssembly()).rejects.toThrow( expect.objectContaining({ transloaditErrorCode: 'RATE_LIMIT_REACHED', + cause: expect.objectContaining({ + error: 'RATE_LIMIT_REACHED', + }), message: 'Response code 413 (Payload Too Large) RATE_LIMIT_REACHED: Request limit reached', }) ) @@ -234,6 +241,9 @@ describe('Mocked API tests', () => { await expect(client.createAssembly()).rejects.toThrow( expect.objectContaining({ transloaditErrorCode: 'IMPORT_FILE_ERROR', + cause: expect.objectContaining({ + error: 'IMPORT_FILE_ERROR', + }), response: expect.objectContaining({ body: expect.objectContaining({ assembly_id: '1' }) }), }) ) @@ -248,7 +258,12 @@ describe('Mocked API tests', () => { .reply(200, { error: 'IMPORT_FILE_ERROR' }) await expect(client.replayAssembly('1')).rejects.toThrow( - expect.objectContaining({ transloaditErrorCode: 'IMPORT_FILE_ERROR' }) + expect.objectContaining({ + transloaditErrorCode: 'IMPORT_FILE_ERROR', + cause: expect.objectContaining({ + error: 'IMPORT_FILE_ERROR', + }), + }) ) scope.done() })