Closed aleclarson closed 5 months ago
@aleclarson Hi, apologies for the delay on this.
So at the moment the TransformDecodeError
and TransformEncodeError
both extend TypeBoxError
(as below)
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 Transform{Decode|Encode}Error
could be updated to the following...
// 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 error.error
(as a sub error property). This would bring the Check and Decode exceptions inline.
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 throw new TransformDecodeError
statement would still be used for non-TB errors (from a user's custom decode function). We just don't want to wrap TB errors with TransformDecodeError
since that makes it impossible to get any metadata attached to the original TransformDecodeCheckError
(in my case, its error: ValueError
property is of particular interest).
@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 path
. Let me know if the updates help out here.
// ------------------------------------------------------------------
// 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.