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

make mocking network errors work for intercepted axios requests #355

Closed
s-pic opened this issue Aug 26, 2020 · 11 comments
Closed

make mocking network errors work for intercepted axios requests #355

s-pic opened this issue Aug 26, 2020 · 11 comments

Comments

@s-pic
Copy link

s-pic commented Aug 26, 2020

Is your feature request related to a problem? Please describe.
I am currently building a httpClient for a browser-based app. I am using msw (it is awesome!) to cover edge-cases like network failures with tests on network level. The client basically wraps axios.
I was looking forward to response.networkError() that was added in #253 because I needed a way to simulate a network problem (like being offline).

I am using networkError(someCustomMessage) in the response resolver. I am asserting that the error caught by axios states my custom message. Instead I am seeing the following error

{
  "message": "Request aborted",
  "code": "ECONNABORTED"
  "stack": "Error: Request aborted
    at createError (D:\dev\kraken\core\node_modules\axios\lib\core\createError.js:16:15)
    at XMLHttpRequestOverride.handleAbort [as onabort] (D:\dev\kraken\core\node_modules\axios\lib\adapters\xhr.js:73:14)
    at XMLHttpRequestOverride.trigger (D:\dev\kraken\core\node_modules\node-request-interceptor\lib\XMLHttpRequest\XMLHttpRequest\createXMLHttpRequestOverride.js:100:92)
    at XMLHttpRequestOverride.abort (D:\dev\kraken\core\node_modules\node-request-interceptor\lib\XMLHttpRequest\XMLHttpRequest\createXMLHttpRequestOverride.js:252:22)
    at D:\dev\kraken\core\node_modules\node-request-interceptor\lib\XMLHttpRequest\XMLHttpRequest\createXMLHttpRequestOverride.js:182:27";
  "//": "..more stuff I stripped from the error"
} 

My error handling logic goes on and treats the error as timeout error, which was not my intention.

Describe the solution you'd like
I would like to augment the simulation of network errors in a way that axios treats the error as network error (error.message === 'Network Error', see axios/axios#383).

I would therefore like to add respective tests to https://github.com/mswjs/msw/tree/master/test/msw-api/res.

Additional context

  1. I can create a minimal reproduction repo if that helps.
  2. I would very much like to contribute here if that is possible.
@s-pic s-pic added the feature label Aug 26, 2020
@s-pic s-pic changed the title Mock network errors make mocking network errors work for intercepted axios requests Aug 26, 2020
@kettanaito
Copy link
Member

Hey, @s-pic. Thanks for the feedback and for reaching out!

Please, could you create a reproduction repo for this? Ideally, with a failed state and an expected state (may be without MSW for now). We'd be happy to help you with this.

@s-pic
Copy link
Author

s-pic commented Aug 27, 2020

Hey @kettanaito thanks for looking into this. I used the examples repo, and in there the rest-react example to reproduce the case in a simplified manner: https://github.com/s-pic/examples.

Here are some instructions to set up and inspect my changes:

  • git clone https://github.com/s-pic/examples
  • cd examples/rest-react/
  • git checkout reproduce_axios_network_error # that is where I worked
  • yarn install && yarn start
  • Visit http://localhost:3001/ and inspect the mocked functionality that uses the added axios-based apiService
  • Run yarn run test:unit --watchAll=false (otherwise the already commited tests are skipped) and inspect the following jest error log:
FAIL src/test/apiService.test.js
  ● apiService › fetchJson › handles a network error

    expect(received).toEqual(expected) // deep equality

    Expected: "Network Error"
    Received: "Request aborted"

      35 |
      36 |             // ASSERT: make sure actual return equals asserted return
    > 37 |             expect(error.message).toEqual(axiosErrorMessage)
         |                                   ^
      38 |         })
      39 |     })
      40 | });

      at Object.<anonymous> (src/test/apiService.test.js:37:35)

PASS src/LoginForm.test.js

Test Suites: 1 failed, 1 passed, 2 total
Tests:       1 failed, 2 passed, 3 total
Snapshots:   0 total
Time:        3.62s, estimated 7s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I figured I should maybe provide some more background:
What I am trying to do is to prepare an axios-based client by writing all kinds of tests for it -- until it is safe to integrate it in the app. I know that using Jest to run tests against axios in a Node environment is not a 100% valid aproach: The client will be used in the browser, but the xhr- and the http- adapter differ in some parts (e.g. regarding timeout handling). But since cypress tests and headless browser tests are not an option at the moment, I kinda have to live with this inaccuracy. Anyways, my goal is to catch all kinds of errors that can occur during a request, detect their nature as good as possible and handle them accordingly.

@s-pic
Copy link
Author

s-pic commented Sep 3, 2020

Hey there, has the provided information been sufficient? Please let me know if I can help to clarify the problem.

@kettanaito
Copy link
Member

Hey, @s-pic. Thanks for providing those clarifications, they are tremendously useful. I'll look into them once I have a minute. Thank you for the patience.

@kettanaito kettanaito self-assigned this Sep 6, 2020
@kettanaito
Copy link
Member

kettanaito commented Sep 6, 2020

Technical insights

The issue happens because the mocked network error is handled differently in a browser and NodeJS. In a browser it's being caught and handled as a special message type sent to the Service Worker:

} catch (error) {
if (error instanceof NetworkError) {
// Treat emulated network error differently,
// as it is an intended exception in a request handler.
return channel.send({
type: 'NETWORK_ERROR',
payload: {
name: error.name,
message: error.message,
},
})
}

In NodeJS, however, there is no such handling, as any thrown exception is handled on the node-request-interceptor library (the library that powers requests interception and mocking in NodeJS). That library handles any exception, including an emulated network error, according to how it should be treated in a regular request.

Here's how an exception is handled in NRI:

https://github.com/mswjs/node-request-interceptor/blob/master/src/interceptors/XMLHttpRequest/XMLHttpRequestOverride.ts#L237-L247

I'm investigating a few possibilities whether this can be changed. Will keep you updated.

@kettanaito
Copy link
Member

It seems that the order of emitted events in XMLHttpRequest override was incorrect:

- abort
- error
+ error
+ abort

Once ordered as error then abort, you receive the given network error message in the axios, and the test passes. The fix is issued in mswjs/interceptors#54, awaits CI and update in MSW.

@kettanaito
Copy link
Member

The fix is published in [email protected]. MSW update to be issued.

@s-pic
Copy link
Author

s-pic commented Sep 7, 2020

This is great @kettanaito !
I wanted to test if the fix makes the test in in the reproduction repo pass, but I am having trouble overriding the transitive dependency. I will test this as soon as I can and get back to you here,

@s-pic
Copy link
Author

s-pic commented Sep 9, 2020

Even if I pin down the transitive dependency of node-request-interceptor to 0.5.1 (by using the resolutions field), I still see the same ouput in the failing test.
See s-pic/examples@1ed266a in the reproduction repo.

Am I missing something here?

@kettanaito You wrote

MSW update to be issued.

I figured that would just include updating the dependency to node-request-interceptor.

@kettanaito
Copy link
Member

This issue was resolved in [email protected]. Could you please update and let us know if it's fixed for you? Thanks.

@s-pic
Copy link
Author

s-pic commented Sep 10, 2020

Yip, that fixed it 😅
Thanks.

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

No branches or pull requests

2 participants