Skip to content

Commit

Permalink
Improve errors alternative implementation (#212)
Browse files Browse the repository at this point in the history 
* 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 <remcohaszing@gmail.com>

* remove stack hack

#211 (comment)

* improve assertion

* fix typo

* fix formatting

---------

Co-authored-by: Remco Haszing <remcohaszing@gmail.com>
  • Loading branch information
@mifi @remcohaszing
mifi and remcohaszing authored Dec 19, 2024
1 parent c6e68f9 commit dc67a98
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 121 deletions.
19 changes: 9 additions & 10 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ try {
}
} catch (err) {
console.error('❌ Unable to process Assembly.', err)
if (err instanceof ApiError && err.response.assembly_id) {
console.error(`💡 More info: https://transloadit.com/assemblies/${err.response.assembly_id}`)
if (err instanceof ApiError && err.assemblyId) {
console.error(`💡 More info: https://transloadit.com/assemblies/${err.assemblyId}`)
}
}
```
Expand Down Expand Up @@ -417,11 +417,10 @@ const url = client.getSignedSmartCDNUrl({

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:

- `HTTPError.response` the JSON object returned by the server. It has these properties
- `error` (`string`) - [The Transloadit API error code](https://transloadit.com/docs/api/response-codes/#error-codes).
- `message` (`string`) - A textual representation of the Transloadit API error.
- `assembly_id`: (`string`) - If the request is related to an assembly, this will be the ID of the assembly.
- `assembly_ssl_url` (`string`) - If the request is related to an assembly, this will be the SSL URL to the assembly .
- `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.:

Expand All @@ -435,15 +434,15 @@ try {
if (err.code === 'ENOENT') {
return console.error('Cannot open file', err)
}
if (err instanceof transloadit.ApiError && err.response.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 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
**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 (`ApiError.response.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
Expand Down
2 changes: 1 addition & 1 deletion examples/retry.js
View file
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 ApiError && err.response.error === 'INVALID_SIGNATURE') {
if (err instanceof ApiError && err.code === 'INVALID_SIGNATURE') {
// This is an unrecoverable error, abort retry
throw new pRetry.AbortError('INVALID_SIGNATURE')
}
Expand Down
40 changes: 15 additions & 25 deletions src/ApiError.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,38 @@ import { HTTPError } from 'got'
export interface TransloaditErrorResponseBody {
error?: string
message?: string
http_code?: string
assembly_ssl_url?: string
assembly_id?: string
}

export class ApiError extends Error {
override name = 'ApiError'

response: TransloaditErrorResponseBody
// 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
appendStack?: string
body: TransloaditErrorResponseBody | undefined
}) {
const { cause, body, appendStack } = params
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)
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)

// if we have a cause, use the stack trace from it instead
if (cause != null && typeof cause.stack === 'string') {
const indexOfMessageEnd = cause.stack.indexOf(cause.message) + cause.message.length
const gotStacktrace = cause.stack.slice(indexOfMessageEnd)
this.stack = `${message}${gotStacktrace}`
}

// If we have an original stack, append it to the bottom, because `got`s stack traces are not very good
if (this.stack != null && appendStack != null) {
this.stack += `\n${appendStack.replace(/^([^\n]+\n)/, '')}`
}

this.response = body ?? {}
this.rawMessage = body.message
this.assemblyId = body.assembly_id
this.assemblySslUrl = body.assembly_ssl_url
this.code = body.error
this.cause = cause
}
}
7 changes: 1 addition & 6 deletions src/Transloadit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ function checkResult<T>(result: T | { error: string }): asserts result is T {
'error' in result &&
typeof result.error === 'string'
) {
throw new ApiError({ body: result }) // in this case there is no `cause` because we don't have a HTTPError
throw new ApiError({ body: result }) // in this case there is no `cause` because we don't have an HTTPError
}
}

Expand Down Expand Up @@ -732,10 +732,6 @@ export class Transloadit {
responseType: 'json',
}

// `got` stacktraces are very lacking, so we capture our own
// https://github.com/sindresorhus/got/blob/main/documentation/async-stack-traces.md
const stack = new Error().stack

try {
const request = got[method]<T>(url, requestOpts)
const { body } = await request
Expand Down Expand Up @@ -766,7 +762,6 @@ export class Transloadit {
) {
throw new ApiError({
cause: err,
appendStack: stack,
body: body as TransloaditErrorResponseBody,
}) // todo don't assert type
}
Expand Down
14 changes: 4 additions & 10 deletions test/integration/live-api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,8 @@ describe('API integration', { timeout: 60000 }, () => {
const promise = createAssembly(client, opts)
await promise.catch((err) => {
expect(err).toMatchObject({
response: expect.objectContaining({
error: 'INVALID_INPUT_ERROR',
assembly_id: expect.any(String),
}),
code: 'INVALID_INPUT_ERROR',
assemblyId: expect.any(String),
})
})
await expect(promise).rejects.toThrow(Error)
Expand Down Expand Up @@ -729,9 +727,7 @@ describe('API integration', { timeout: 60000 }, () => {
expect(ok).toBe('TEMPLATE_DELETED')
await expect(client.getTemplate(templId!)).rejects.toThrow(
expect.objectContaining({
response: expect.objectContaining({
error: 'TEMPLATE_NOT_FOUND',
}),
code: 'TEMPLATE_NOT_FOUND',
})
)
})
Expand Down Expand Up @@ -802,9 +798,7 @@ describe('API integration', { timeout: 60000 }, () => {
expect(ok).toBe('TEMPLATE_CREDENTIALS_DELETED')
await expect(client.getTemplateCredential(credId!)).rejects.toThrow(
expect.objectContaining({
response: expect.objectContaining({
error: 'TEMPLATE_CREDENTIALS_NOT_READ',
}),
code: 'TEMPLATE_CREDENTIALS_NOT_READ',
})
)
})
Expand Down
121 changes: 52 additions & 69 deletions test/unit/mock-http.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,8 @@ describe('Mocked API tests', () => {

await expect(client.createAssembly()).rejects.toThrow(
expect.objectContaining({
response: {
error: 'INVALID_FILE_META_DATA',
message: 'Invalid file metadata',
},
code: 'INVALID_FILE_META_DATA',
rawMessage: 'Invalid file metadata',
message: 'API error (HTTP 400) INVALID_FILE_META_DATA: Invalid file metadata',
})
)
Expand All @@ -122,64 +120,55 @@ describe('Mocked API tests', () => {
expect.objectContaining({
message:
'API error (HTTP 400) INVALID_FILE_META_DATA: Invalid file metadata https://api2-oltu.transloadit.com/assemblies/foo',
response: expect.objectContaining({ assembly_id: '123' }),
assemblyId: '123',
})
)

try {
await promise
} catch (err) {
expect(inspect(err).split('\n')).toEqual([
expect.stringMatching(
`API error \\(HTTP 400\\) INVALID_FILE_META_DATA: Invalid file metadata https://api2-oltu.transloadit.com/assemblies/foo`
),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(
` at Transloadit\\._remoteJson \\(.+\\/src\\/Transloadit\\.ts:\\d+:\\d+\\)`
),
expect.stringMatching(
` at createAssemblyAndUpload \\(.+\\/src\\/Transloadit\\.ts:\\d+:\\d+\\)`
),
expect.stringMatching(` at .+\\/src\\/Transloadit\\.ts:\\d+:\\d+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+\\/test\\/unit\\/mock-http\\.test\\.ts:\\d+:\\d+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` name: 'ApiError',`),
expect.stringMatching(` response: \\{`),
expect.stringMatching(` error: 'INVALID_FILE_META_DATA',`),
expect.stringMatching(` message: 'Invalid file metadata',`),
expect.stringMatching(` assembly_id: '123',`),
expect.stringMatching(
` assembly_ssl_url: 'https:\\/\\/api2-oltu\\.transloadit\\.com\\/assemblies\\/foo'`
),
expect.stringMatching(` \\},`),
expect.stringMatching(` cause: HTTPError: Response code 400 \\(Bad Request\\)`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` code: 'ERR_NON_2XX_3XX_RESPONSE',`),
// don't care about the rest:
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.stringMatching(' }'),
expect.stringMatching(' }'),
expect.stringMatching('}'),
])
}
const errorString = await promise.catch(inspect)
expect(typeof errorString === 'string').toBeTruthy()
expect(inspect(errorString).split('\n')).toEqual([
expect.stringMatching(
`API error \\(HTTP 400\\) INVALID_FILE_META_DATA: Invalid file metadata https://api2-oltu.transloadit.com/assemblies/foo`
),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(
` at createAssemblyAndUpload \\(.+\\/src\\/Transloadit\\.ts:\\d+:\\d+\\)`
),
expect.stringMatching(` at .+\\/test\\/unit\\/mock-http\\.test\\.ts:\\d+:\\d+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` rawMessage: 'Invalid file metadata',`),
expect.stringMatching(` assemblyId: '123',`),
expect.stringMatching(
` assemblySslUrl: 'https:\\/\\/api2-oltu\\.transloadit\\.com\\/assemblies\\/foo'`
),
expect.stringMatching(` code: 'INVALID_FILE_META_DATA',`),
expect.stringMatching(` cause: HTTPError: Response code 400 \\(Bad Request\\)`),
expect.stringMatching(` at .+`),
expect.stringMatching(` at .+`),
expect.stringMatching(` code: 'ERR_NON_2XX_3XX_RESPONSE',`),
// don't care about the rest:
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.anything(),
expect.stringMatching(' }'),
expect.stringMatching(' }'),
expect.stringMatching('}'),
])
})

it('should retry correctly on RATE_LIMIT_REACHED', async () => {
Expand Down Expand Up @@ -211,9 +200,7 @@ describe('Mocked API tests', () => {
await expect(client.createAssembly()).rejects.toThrow(
expect.objectContaining({
message: 'API error (HTTP 413) RATE_LIMIT_REACHED: Request limit reached',
response: expect.objectContaining({
error: 'RATE_LIMIT_REACHED',
}),
code: 'RATE_LIMIT_REACHED',
})
)
scope.done()
Expand Down Expand Up @@ -292,10 +279,8 @@ describe('Mocked API tests', () => {

await expect(client.createAssembly()).rejects.toThrow(
expect.objectContaining({
response: expect.objectContaining({
error: 'IMPORT_FILE_ERROR',
assembly_id: '1',
}),
code: 'IMPORT_FILE_ERROR',
assemblyId: '1',
})
)
scope.done()
Expand All @@ -310,9 +295,7 @@ describe('Mocked API tests', () => {

await expect(client.replayAssembly('1')).rejects.toThrow(
expect.objectContaining({
response: expect.objectContaining({
error: 'IMPORT_FILE_ERROR',
}),
code: 'IMPORT_FILE_ERROR',
})
)
scope.done()
Expand Down

0 comments on commit dc67a98

Please sign in to comment.