-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: rethrow TypeBoxError instead of creating a new one #777
Conversation
Valuable information is lost by creating a TransformEncodeError/TransformDecodeError instead of rethrowing the error when it‘s a TypeBoxError, because that error may be a TransformEncodeCheckError/TransformDecodeCheckError, which have an `error` property that tells the caller which property path is the cause.
@aleclarson Hi, apologies for the delay on this. So at the moment the export class TransformDecodeError extends TypeBoxError { // <-- here
constructor(public readonly schema: TSchema, public readonly value: unknown, error: any) {
super(`${error instanceof Error ? error.message : 'Unknown error'}`)
}
} Which should mean the following condition would always be true. //...
} catch (error) {
if (error instanceof TypeBoxError) throw error // expect: always true
throw new TransformDecodeError(schema, value, error)
} However, the I see that the constructor signature for // current
constructor(public readonly schema: TSchema, public readonly value: unknown, error: any) {
super(`${error instanceof Error ? error.message : 'Unknown error'}`)
}
// updated
constructor(public readonly schema: TSchema, public readonly value: unknown, public readonly error: any) {
super(`${error instanceof Error ? error.message : 'Unknown error'}`) // ^ update
} which would mean the actual error would be obtainable via import { Value, TransformDecodeError, TransformDecodeCheckError } from '@sinclair/typebox/value'
import { Type } from '@sinclair/typebox'
{ // TransformDecodeCheckError
const T = Type.Transform(Type.String())
.Decode(value => parseInt(value))
.Encode(value => value.toString())
try {
Value.Decode(T, null)
} catch(error) {
if(error instanceof TransformDecodeCheckError) {
console.log(error.schema) // the schema
console.log(error.error) // value error
}
}
}
{ // TransformDecodeError
const T = Type.Transform(Type.String())
.Decode(value => { throw Error('fail to decode') })
.Encode(value => { throw Error('fail to encode') })
try {
Value.Decode(T, 'null')
} catch(error) {
if(error instanceof TransformDecodeError) {
console.log(error.schema) // the schema
console.log(error.error) // thrown error.
}
}
} Would this help solve the issue? Also, I note that the exception path isn't included any of the decode / encode exception types, this could possibly be included via separate PR (as exception paths would require quite a bit more work to obtain) Let me know your thoughts! |
The existing |
@aleclarson Heya I think this one can be closed off. I've updated the Transform Error types on 0.32.16 to include the exception that was thrown. I've also updated the Decode/Encode exceptions specifically to include a // ------------------------------------------------------------------
// Errors
// ------------------------------------------------------------------
// thrown externally
// prettier-ignore
export class TransformDecodeCheckError extends TypeBoxError {
constructor(
public readonly schema: TSchema,
public readonly value: unknown,
public readonly error: ValueError // note: path derived from ValueError
) {
super(`Unable to decode value as it does not match the expected schema`)
}
}
// prettier-ignore
export class TransformDecodeError extends TypeBoxError {
constructor(
public readonly schema: TSchema,
public readonly path: string, // added: path to where Decode callback failed
public readonly value: unknown,
public readonly error: Error, // added: the exception that was thrown
) {
super(error instanceof Error ? error.message : 'Unknown error')
}
} I do think it might be good to try and unify these two errors at some point (at least get the path properties lining up), but this update just augments the existing error types to include additional information. Happy to review general API DX updates with respect to error handling via GH issues/discussion (can discuss things there first) Thanks again! |
Valuable information is lost by creating a TransformEncodeError/TransformDecodeError instead of rethrowing the error when it‘s a TypeBoxError, because that error may be a TransformEncodeCheckError/TransformDecodeCheckError, which have an
error
property that tells the caller which property path is the cause.