buildAuthenticatedFetch no longer supports custom fetch as a first argument

[mrkvon]

This is either bug or (regressed) feature request.

Search terms you've used

buildAuthenticatedFetch

Impacted package

Which packages do you think might be impacted by the bug ?

• [x] solid-client-authn-core

Description

It looks like since v2, the library no longer supports passing custom fetch to buildAuthenticatedFetch (used to be first argument). That can be considered a breaking change and feature regression. Please note that others depend directly on buildAuthenticatedFetch, and passing custom fetch can be very useful. In my case building authenticated request in Cypress.

Expected result

It would be awesome if we can continue passing custom fetch to buildAuthenticatedFetch (e.g. as an option). I don't mind if API changes (e.g. passing fetch as optional property in second parameter).

[NSeydoux]

Thanks for reporting this. It is indeed a braking change, although it wasn't documented as such because we don't document @inrupt/solid-client-authn-core as public API, it is used internally by our other libraries. It is good to know that this is a use case for community members, we will consider this for future changes and releases so that this doesn't happen again.

I can't commit to a timeline for this to be changed in a way that suits your use case, so in the interim would it be possible to inverse the responsibilities and build the customized fetch from the authenticated one? Something a bit like https://github.com/inrupt/solid-client-access-grants-js/blob/f074a59dd61b45bbaf4c6fc811ffd419626049ae/e2e/node/e2e.test.ts#L149, where you pass in the customizations and use the output of buildAuthenticatedFetch as a base?

If that doesn't work, I'm happy to assist making the change. Things were modified in https://github.com/inrupt/solid-client-authn-js/commit/494af00b5e92e80ff3ed47b6221bb781ea93be5d, where you'll notice the unauthFetch parameter was removed. If you add it back into the options, it will be a non-breaking change (from a library standpoint) that will make it possible to fix the regression with a minimal code change on your end. What do you think?

[mrkvon]

FYI CSS guide Automating authentication with Client Credentials depends on buildAuthenticatedFetch. It's the reason why I use it as well, both for sending authenticated requests in cypress (testing) and css-authn library.

[mrkvon]

Thank you for the elaborate reply! 🙏🏼

My use case here is to send authenticated requests during testing to CSS with cy.request, which is copied from the ingenious work by folks of SolidCryptPad. I haven't found a way to wrap cy.request around standard fetch, and I'm not sure it's possible.

For now, I have copied and edited the relevant file to the affected codebase. So, this is not urgent.

It would be indeed helpful if breaking changes to important core API get documented in the future. OTOH if fetch becomes a second-argument options' optional parameter, it won't have to be a breaking change. 🙂

Thank you for this amazing library! 🤩

[michielbdejong]

would it be possible to inverse the responsibilities and build the customized fetch from the authenticated one?

~~Yes, that would work for some use cases, but no, unfortunately not for all. In particular, I want to pass a custom dispatcher to undici fetch so I can set the body timeout to infinite. This is necessary for listening to a StreamingHTTPChannel2023 stream.~~

~~Please advise, thank you!~~

EDIT: actually I was able to fix my use case in a much simpler way, so not necessary after all, in my case, to pass a custom fetch:

fetch(streamingUrl, {
  dispatcher: new Agent({bodyTimeout: 0})
} as RequestInit);