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

fetch adapter: handling of network errors different from xhr #6374

Closed
glsignal opened this issue Apr 29, 2024 · 1 comment · Fixed by #6380
Closed

fetch adapter: handling of network errors different from xhr #6374

glsignal opened this issue Apr 29, 2024 · 1 comment · Fixed by #6380

Comments

@glsignal
Copy link

glsignal commented Apr 29, 2024

Describe the bug

Hello! First up: thank you for the work to include a fetch adapter in axios, this is fantastic ❤️

I noticed a (potential) issue, or at least a difference in behaviour, when switching adapters around the production of ERR_NETWORK.

Glancing at the fetch adapters code, it looks like the issue comes from the if (err.name === 'NetworkError') { check, which works with XHR but fetch rejects with a TypeError with Failed to fetch as the message. It might just be a matter of swapping the strings, in which case that's straight forward.

What I wasn't sure about, is how wide that "Failed to fetch" scope is (MDN only describes the exception class), and whether it overlaps with any of the other error types that are listed. MDN documents a number of failure modes for this TypeError, which may complicate things.

Happy to help/open a PR when needed, though I'd like to iron out the uncertainty around the error modes first

To Reproduce

https://github.com/glsignal/scratch-axios-network-error or https://codesandbox.io/p/github/glsignal/scratch-axios-network-error/main

Code snippet

import { beforeAll, afterAll, describe, expect, it } from "vitest";

import { http, HttpResponse } from "msw";
import { setupServer } from "msw/node";

import axios, { AxiosError } from "axios";

const server = setupServer();

beforeAll(() => {
  server.listen();
});

afterAll(() => {
  server.close();
});

describe("error events", () => {
  async function buildNetworkError(adapter) {
    server.use(http.get("/test", () => HttpResponse.error()));

    try {
      return await axios.get("http://localhost/test", { adapter });
    } catch (event) {
      return event;
    }
  }

  describe("using xhr adapter", () => {
    it("indicates that it is a network error", async () => {
      const error = await buildNetworkError("xhr");

      expect(error.code).toEqual(AxiosError.ERR_NETWORK);
    });
  });

  describe("using fetch adapter", () => {
    it("indicates that it is a network error", async () => {
      const error = await buildNetworkError("fetch");

      expect(error.code).toEqual(AxiosError.ERR_NETWORK);
    });
  });
});

Expected behavior

I'd expect error.code to equal AxiosError.ERR_NETWORK with both adapters.

Axios Version

1.7.0-beta.0

Adapter Version

fetch

Browser

No response

Browser Version

No response

Node.js Version

No response

OS

No response

Additional Library Versions

No response

Additional context/Screenshots

No response

@DigitalBrainJS
Copy link
Collaborator

DigitalBrainJS commented May 2, 2024

Yes, it wouldn't be easy. Errors causes are not standardized, they are different in browser/node(undici/cross-fetch/etc)/ReactNative.
Writing an error parser for each platform can hardly be called an acceptable solution, especially since there are no guarantees that there are no differences between versions. For now, I'm inclined to consider TypeError: Failed to fetch as an AxiosError with ERR_NETWORK code. Maybe someone will come up with a better solution.

P.S. Thanks for the test example & repo. It's rare to see such a well-filled issue. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants