Skip to content
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

Add error serializer #3655

Closed
4 tasks done
lukaselmer opened this issue Jun 23, 2023 · 5 comments · Fixed by #4396
Closed
4 tasks done

Add error serializer #3655

lukaselmer opened this issue Jun 23, 2023 · 5 comments · Fixed by #4396

Comments

@lukaselmer
Copy link

lukaselmer commented Jun 23, 2023

Clear and concise description of the problem

Custom errors with additional attributes are hard to test using toThrowErrorMatchingInlineSnapshot

Example:

class ErrorWithDetails extends Error {
  readonly details: unknown

  constructor(message: string, options: ErrorOptions & { details: unknown }) {
    super(message, options)
    this.details = options.details
  }
}

describe('ErrorWithDetails', () => {
  it('should throw an error with details', () => {
    expect(() => {
      throw new ErrorWithDetails('Example', { details: 'interesting detail' })
    }).toThrowErrorMatchingInlineSnapshot('"Example"') // 'interesting detail' is lost
  })
})

Suggested solution

Add an addErrorSerializer which works like addSnapshotSerializer, but for errors.

Example:

expect.addErrorSerializer({
  serialize(val, config, indentation, depth, refs, printer) {
    const error = val as ErrorWithDetails
    return `${error.message}: ${printer(error.details, config, indentation, depth, refs)}`
  },
  test(val) {
    return val && val instanceof ErrorWithDetails
  },
})

which could then result in:

class ErrorWithDetails extends Error {
  readonly details: unknown

  constructor(message: string, options: ErrorOptions & { details: unknown }) {
    super(message, options)
    this.details = options.details
  }
}

describe('ErrorWithDetails', () => {
  it('should throw an error with details', () => {
    expect(() => {
      throw new ErrorWithDetails('Example', { details: 'interesting detail' })
    }).toThrowErrorMatchingInlineSnapshot('"Example: interesting detail"')
  })
})

Alternative

We can work around that issue, but it is cumbersome:

class ErrorWithDetails extends Error {
  readonly details: unknown

  constructor(message: string, options: ErrorOptions & { details: unknown }) {
    super(message, options)
    this.details = options.details
  }
}

describe('ErrorWithDetails', () => {
  it('should throw an error with details', () => {
    expect(
      catchAndSerializeError(() => {
        throw new ErrorWithDetails('Example', { details: 'interesting detail' })
      })
    ).toMatchInlineSnapshot('"Example \\"interesting detail\\""')
  })
})

function catchAndSerializeError(fn: () => any) {
  try {
    fn()
    throw new Error('Expected an error')
  } catch (error) {
    if (error instanceof ErrorWithDetails) {
      return `${error.message} ${JSON.stringify(error.details)}`
    }
    throw new Error('Expected an ErrorWithDetails')
  }
}

Additional context

No response

Validations

@sheremet-va
Copy link
Member

Can't you just use addSnapshotSerializer?

@lukaselmer
Copy link
Author

lukaselmer commented Jun 27, 2023

Can't you just use addSnapshotSerializer?

I was hoping that, but no, that didn't work 😥

I think I also found the place where the error is converted, and there it's just if(error instanceof Error) return error.message

@sheremet-va
Copy link
Member

Looks like a bug. I would assume it to be affected

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Oct 30, 2023

Hi, I'm curious about this issue and I'd like to confirm what's expected to work or not work.

I'm experimenting with OP's example locally and I put reproduction at the bottom.
In summary, custom serializer seems to work like this currently:

  • expect(someError).toMatchInlineSnapshot(...)
    • Serializer applied
  • expect(() => { throw someError }).toThrowErrorMatchingInlineSnapshot(...)
    • Serializer not applied. Snapshot will become someError.message.

Could someone confirm whether custom serializer should be applied for 2nd case as well?

Given that first case works, I also included potential workaround for users in the snippet below.

reveal full snippet
test('custom serializer against thrown instance', async () => {
  class ErrorWithDetails extends Error {
    readonly details: unknown

    constructor(message: string, options: ErrorOptions & { details: unknown }) {
      super(message, options)
      this.details = options.details
    }
  }

  // without custom serializer
  const error = new ErrorWithDetails('Example', { details: 'interesting detail' })
  expect(() => {
    throw error
  }).toThrowErrorMatchingInlineSnapshot(`"Example"`)
  expect(error).toMatchInlineSnapshot(`[Error: Example]`)

  // with custom serializer
  expect.addSnapshotSerializer({
    serialize(val, config, indentation, depth, refs, printer) {
      const error = val as ErrorWithDetails
      return `${error.message}: ${printer(error.details, config, indentation, depth, refs)}`
    },
    test(val) {
      return val && val instanceof ErrorWithDetails
    },
  })
  expect(() => {
    throw error
  }).toThrowErrorMatchingInlineSnapshot(`"Example"`) // serializer not applied
  expect(error).toMatchInlineSnapshot(`Example: "interesting detail"`) // serializer applied

  //
  // workaround 1 (for async error)
  //   by unwrapping with `rejects, it can assert error instance via `toMatchInlineSnapshot`
  //
  await expect(async () => {
    throw error
  }).rejects.toMatchInlineSnapshot(`Example: "interesting detail"`)

  //
  // workaround 2
  //   create a utility to catch error and use it to assert snapshot via `toMatchInlineSnapshot`
  //
  function wrapError<T>(f: () => T): { success: true; data: T } | { success: false; error: unknown } {
    try {
      return { success: true, data: f() }
    }
    catch (error) {
      return { success: false, error }
    }
  }
  expect(wrapError(() => {
    throw error
  })).toMatchInlineSnapshot(`
    {
      "error": Example: "interesting detail",
      "success": false,
    }
  `)
})

While looking around the code, it looks like Error.message extraction for toThrowErrorMatchingInlineSnapshot happens here:

getSnapshotClient().assert({
received: getErrorString(expected, promise),

But changing here might be a huge breaking change

function getErrorString(expected: () => void | Error, promise: string | undefined) {
  ...
  catch (e) {
-    return getErrorMessage(e)    
+    return e
  }
  ...
}

since this will lead to a different default throw snapshot:

// before
  expect(() => {
    throw new Error('omega')
  }).toThrowErrorMatchingInlineSnapshot(`"ometa"`)

// after
  expect(() => {
    throw new Error('omega')
  }).toThrowErrorMatchingInlineSnapshot(`[Error: omega]`)

EDIT: I created a draft PR as it might help the discussion better with concrete code. #4396

@lukaselmer
Copy link
Author

@hi-ogawa I came to exactly the same conclusion. Before creating a PR, I wanted to confirm that this is really an issue. Thanks for creating the PR, looks promising!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants