kettanaito
commented
8 months ago Hi, @nzakas. Thanks for suggesting this.
Just throw the original error. I'm not sure what benefit wrapping the error in another error provides, but it seems like throwing the original error would be more useful.
The reason why throw a generic Failed to fetch is to be compliant with how the native fetch handles request errors. We value that compliance a lot because the way your test behaves with and without the mock must be close to identical. Without the mock, a request error will always result in a generic Failed to fetch error. This is just how the Fetch API behaves, and we should respect that.
Not to mention, any custom properties on the thrown error aren't accessible on the wrapped error. That means the nice output that assertion errors typically create in the console are lost.
Just like the native fetch, we preserve the actual error thrown in the error.cause property. Your assertion errors will be visible there.
If you must wrap the original error, at least append the original error message after "Failed to fetch", so we can see "Failed to fetch: My original error message".
This changes the type of the error and deviates the fetch behavior compared to when you aren't using Interceptors. I don't believe this is the way forward.
How we can improve this
I agree that's a suboptimal developer experience. On our end, I think to improve the developer experience, we can forward the original error to the console alongside the actual generic error:
console.error(originalError)
throw new TypeError('Failed to fetch', { cause: originalError })However, we cannot reliably reason about which errors you wish to see and which you don't. If you are mocking intentional server-side errors, you don't want to see those repeated in your console during the test run. This makes it hard for us to differentiate between these two errors, and this is the reason why I don't think we should do anything about them implicitly.
How you can improve this
On your end, you shouldn't put assertions in request handlers. Remember that you write request handlers from the server's perspective. This means that any error thrown in a handler is treated as an error thrown in the actual server processing your request. Naturally, it translates to the network.
Instead, capture the resolver call and assert on the mock:
const resolverListener = vi.fn()
server.use(http.get(url, resolverListener))
await waitFor(() => expect(resolverListener).toHaveBeenCalled())
expect(resolverListener).toHaveBeenCalledWith(
expect.objectContaining({
params: { folder_id: expectedFolderId },
// ...other assertions.
})
)To assert more complex structures, using asymmetric assertions can be tedious. Instead, you can unwrap the call to the mock function and assert on the data itself:
const { params, request } = resolverListener.mock.calls[0][0]
// ...assertions.This is how I recommend solving this problem:
- Don't mix network behavior and the test logic.
- Write assertions independently of their source (the mock function).
nzakas
Right now, any error that occurs inside of an interceptor is wrapped in a network error. Depending on the system that is receiving the error, this can cause problems.
In my case, I'm running asserts inside of an interceptor, like this:
When any of the asserts fail, Mocha just displays "Failed to fetch", and that really destroys my productivity while debugging.
Not to mention, any custom properties on the thrown error aren't accessible on the wrapped error. That means the nice output that assertion errors typically create in the console are lost.
Suggestions