fix: rethrow TypeBoxError instead of creating a new one

[aleclarson]

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.

[sinclairzx81]

@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!

[aleclarson]

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).

[sinclairzx81]

@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!