search

knee-cola / jest-mock-axios

Axios mock for Jest
252 stars 42 forks source link

Add `isAxiosError` #66

Closed mAAdhaTTah closed 3 years ago

mAAdhaTTah commented 3 years ago

Uses the same isAxiosError implementation from axios proper. Modifies any thrown errors to add an isAxiosError property for use by this function.

Fixes #65.

mAAdhaTTah commented 3 years ago

This seemed like the "cheapest" way to do it but the main issue here is an Axios error has other information attached to the error object, which is the main reason you'd want to check if it's an Axios error. This implementation doesn't do anything to build up that error object so it's possible that downstream consumer code will fail when it attempts to pluck the .response property from the error.

Not 100% sure how to solve without implementing like a MockError object or something of that nature, like was done w/ the Cancel tokens. Open to suggestions on how to proceed here (if we should at all).

kingjan1999 commented 3 years ago

@mAAdhaTTah Thank you very much for this PR! While I agree that this way is probably the cheapest way, I wonder if it's the best one, as it would add yet another instance of global state here.

What do you think about copying the implementation for isAxiosError from axios directly and just adding the isAxiosError property to any errors passed to mockError() (maybe add a check to see if the error object already contains isAxiosError and not overwrite then)? (Maybe something similar to this patch) That would be quite as cheap as the previous solution whilst also providing support for everyone not using the isAxiosError helper function and looking directly add the property instead.

Concerning the .response property: I think it's up to the individual test code to equip the error passed to mockError with all properties needed in the test code.

mAAdhaTTah commented 3 years ago

@kingjan1999 Sounds like a plan! Will get this PR updated.

Concerning the .response property: I think it's up to the individual test code to equip the error passed to mockError with all properties needed in the test code.

Makes sense.

mAAdhaTTah commented 3 years ago

(Clicked close by accident)

mAAdhaTTah commented 3 years ago

@kingjan1999 This is ready.

kingjan1999 commented 3 years ago

LGTM - thank you very much!