microsoft / kiota-typescript

TypeScript libraries for Kiota-generated API clients.
https://aka.ms/kiota/docs
MIT License
34 stars 25 forks source link

v1.12.0: Failed to execute 'fetch' on 'Window': Illegal invocation #1098

Closed seniorquico closed 5 months ago

seniorquico commented 6 months ago

We upgraded our web-based, Vite/React SPA to the new Kiota v1.12.0 and upgraded our generated API client. The dependencies in our project were updated to include (as dictated by kiota info ...):

@microsoft/kiota-abstractions: 1.0.0-preview.44
@microsoft/kiota-http-fetchlibrary: 1.0.0-preview.43
@microsoft/kiota-serialization-json: 1.0.0-preview.44

Running the updated project, API requests fail with the following message and stack:

TypeError: Failed to execute 'fetch' on 'Window': Illegal invocation
    at CustomFetchHandler.<anonymous> (http://localhost:3000/node_modules/.vite/deps/@microsoft_kiota-http-fetchlibrary.js?v=8cce6e32:598:29)
    at Generator.next (<anonymous>)
    at http://localhost:3000/node_modules/.vite/deps/@microsoft_kiota-http-fetchlibrary.js?v=8cce6e32:185:67
    at new Promise (<anonymous>)
    at Object.__awaiter (http://localhost:3000/node_modules/.vite/deps/@microsoft_kiota-http-fetchlibrary.js?v=8cce6e32:167:10)
    at CustomFetchHandler.execute (http://localhost:3000/node_modules/.vite/deps/@microsoft_kiota-http-fetchlibrary.js?v=8cce6e32:597:24)
    at HeadersInspectionHandler.<anonymous> (http://localhost:3000/node_modules/.vite/deps/@microsoft_kiota-http-fetchlibrary.js?v=8cce6e32:1600:44)
    at Generator.next (<anonymous>)
    at http://localhost:3000/node_modules/.vite/deps/@microsoft_kiota-http-fetchlibrary.js?v=8cce6e32:185:67
    at new Promise (<anonymous>)

Our client initialization code has been:

const myApiClient = createMyApiClient(new FetchRequestAdapter(new AnonymousAuthenticationProvider()))

Modifying the client initialization to use an explicitly bound window.fetch function "fixes" the error and gets us back to a working integration:

const myApiClient = createMyApiClient(new FetchRequestAdapter(new AnonymousAuthenticationProvider(), undefined, undefined, KiotaClientFactory.create(window.fetch.bind(window))))

Are we initializing our API client correctly? Or is this an unintentional issue with Kiota v1.12.0?

(Edit: And I should note- downgrading to the previous version of Kiota also works.)

baywet commented 6 months ago

Hi @seniorquico Thanks for using kiota and for reaching out. Yes the initialization is correct as far as I can tell. It's strange that your tests can't find fetch which is supposed to be defined both on global (that's the one being referenced) and on window. We had removed an older version of node-fetch as a dependency, for sanity we cleaned it up with #1092 but for some reason we forgot to publish that change. I've just released new versions with #1100. Can you upgrade and see whether you still need the workaround please?

seniorquico commented 6 months ago

@baywet I upgraded to the following dependency versions and reverted the initialization code, but I still get the the "illegal invocation" error. The workaround still works with these new versions, as well.

@microsoft/kiota-abstractions: 1.0.0-preview.45
@microsoft/kiota-http-fetchlibrary: 1.0.0-preview.44
@microsoft/kiota-serialization-json: 1.0.0-preview.45

Very odd!

seniorquico commented 6 months ago

@baywet And just to confirm- reverting to Kiota v1.12.0-preview.202402220002, the following dependency versions, and the original initialization code (no explicit window.fetch binding to window) works as expected- no "illegal invocation" error.

@microsoft/kiota-abstractions: 1.0.0-preview.43
@microsoft/kiota-http-fetchlibrary: 1.0.0-preview.42
@microsoft/kiota-serialization-json: 1.0.0-preview.43

This was what we had been testing with prior to upgrading to Kiota v1.12.0.

baywet commented 6 months ago

Thanks for the additional information. And you're seeing that behavior only in vitest, not when running the code in browser and getting the data from the actual API?

when looking at the changes between both versions the only things that jump to mind are default values for the middlewareFactory and httpClient, I suspect that somehow an undefined is being kept instead if using parameters default values.

Maybe the httpClient constructor should get defaulted with fetch as any like other places do.

Regardless, I think a regression was introduced, if a custom is in fact passed to the constructor parameter, we'll end up with two CustomFetchHandlers and I think the body of the constructor should be mostly reverted to what it was besides the renames.

And as for the default values, maybe fetch ?? (window && window.fetch) as any instead (or some variation of that) would be better adapter (we should also include global.fetch for workers then).

Do you think you could explore those avenues by cloning the repository locally and testing those changes?

seniorquico commented 6 months ago

@baywet No, this is happening in both development & production builds of the app in the web browser. I actually haven't tested it yet with vitest.

Happy to try a local, modified build. This may take a few days, but I'll get back as soon as I can.

Jannik-KJ commented 6 months ago

I'm experiencing this error as well using the latest version 1.12.0 and latest preview of the different npm's

baywet commented 5 months ago

Thanks everyone for the additional input. It helps a lot. I'm currently traveling for a conference, so I won't have time to look into that before next week at the very best. If one (or more) of you could get the investigation started locally and start putting together a PR, it'd help fast-track things.

Jannik-KJ commented 5 months ago

Adding this to the requestAdaptor KiotaClientFactory.create(window.fetch.bind(window)) also solved it for me as a temp solution

seniorquico commented 5 months ago

I just tested with Kiota 1.13.0-preview.202403280001 and manually updated to the following:

@microsoft/kiota-abstractions: 1.0.0-preview.50
@microsoft/kiota-http-fetchlibrary: 1.0.0-preview.49
@microsoft/kiota-serialization-json: 1.0.0-preview.50

I can confirm it's fixed and working without the explicit bind. Thanks, all!