Skip to content

Commit

Permalink
improve TransloaditError (#210)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
mifi authored Dec 12, 2024
1 parent 3f1acb0 commit 0e0e9e1
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 28 deletions.
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
}
}
})()
Expand Down Expand Up @@ -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.:
Expand All @@ -435,15 +435,15 @@ 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)
}
}
```
**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
Expand Down
2 changes: 1 addition & 1 deletion examples/retry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
Expand Down
58 changes: 42 additions & 16 deletions src/Transloadit.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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'

Expand Down Expand Up @@ -47,32 +47,58 @@ interface CreateAssemblyPromise extends Promise<Assembly> {
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
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 */
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
Expand All @@ -95,7 +121,7 @@ function checkResult<T>(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)
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
28 changes: 26 additions & 2 deletions src/TransloaditError.ts
Original file line number Diff line number Diff line change
@@ -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 }
}
Expand Down
18 changes: 16 additions & 2 deletions test/integration/live-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
})
})
Expand Down Expand Up @@ -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',
}),
})
)
})
})
Expand Down Expand Up @@ -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',
}),
})
)
})
})
Expand Down
17 changes: 16 additions & 1 deletion test/unit/mock-http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
})
)
Expand All @@ -119,6 +122,7 @@ describe('Mocked API tests', () => {
response: expect.objectContaining({
body: expect.objectContaining({ assembly_id: '123' }),
}),
cause: expect.objectContaining({ assembly_id: '123' }),
})
)
})
Expand Down Expand Up @@ -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',
})
)
Expand Down Expand Up @@ -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' }) }),
})
)
Expand All @@ -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()
})
Expand Down

0 comments on commit 0e0e9e1

Please sign in to comment.