immers-space / activitypub-express

Modular ActivityPub implementation as Express JS middleware to easily add decentralization and federation to Node apps
MIT License
295 stars 22 forks source link

Request library deprecated #12

Open wmurphyrd opened 3 years ago

wmurphyrd commented 3 years ago

Find a new library for sending http requests

wmurphyrd commented 3 years ago

this looks promising: https://github.com/blocktrail/superagent-http-signature

However it's forked from an hold version of http-signature so it will almost certainly need the patches we're currently applying to http-signature added to it

gregid commented 3 years ago

I have played a bit with request-compose over the weekend and I must say I really like it for it's flexibility/customization.

I've made draft version of request with this library and http-signature-header for signature (as discussed in #40): https://gist.github.com/gregid/e49f041ac5e8d857edec6cc6d2f0b19b

Generating signatures is done but I still have problems with verification of the generated signature so I may have made some mistakes as this is my first time dealing with HTTP Signatures.

But overall I do think request-compose is a good fit for replacement especially if considering other enhancements, like json-ld signatures in the future.

wmurphyrd commented 3 years ago

@gregid this looks great! Can you start a pull request and we'll work out the kinks together?

gregid commented 3 years ago

Will do - I didn't want to impose this without your interest.

ThisIsMissEm commented 1 year ago

@wmurphyrd I'd perhaps be inclined to look towards Node 18's fetch support, which is undici under the hood (requires at least node 16)

Then you'd just need the ability to take a Request / Response object and do http-signatures on it, potentially even allowing setting fetch on the apex instance, this would allow "bring your own fetch implementation", which would also help in unit testing the code.

wmurphyrd commented 1 year ago

@ThisIsMissEm oh now this is interesting. I'd glanced at node's fetch when it was announced, but didn't think it would work here because of the need to manipulate an existing Request object. I just looked again based on your prompting and realized that all these years using fetch in the browser, I'd never noticed it's possible to construct a Request first and then pass it to fetch.

I also just created an interface mock for http-signature to use in the unit tests, so I'm pretty certain I could coerce http-signature to work with a Request

https://developer.mozilla.org/en-US/docs/Web/API/fetch

wmurphyrd commented 1 year ago

@ThisIsMissEm well, converting the code to fetch was simple, but unfortunately it does not play well with the unit tests:

So, no idea if it actually works, and refactoring request mocking for the test suites would be a massive chore. Looks like native fetch is out at least for now

ThisIsMissEm commented 1 year ago

Ah, yeah, we've seen similar issue with the question of "how to mock fetch in node.js 18" in the @Inrupt Solid SDKs, the approach we're taking is to shim it, so we'll have a module that either directs to fetch or to the mock.

It's still early days for us, but we're going to have to rewrite all our unit tests to this new setup away from module mocking cross-fetch.

Once we've something working, I kinda want to take it to WinterCG to propose a standardised way to mock fetch.

wmurphyrd commented 1 year ago

Ah, yeah, we've seen similar issue with the question of "how to mock fetch in node.js 18" in the @inrupt Solid SDKs, the approach we're taking is to shim it, so we'll have a module that either directs to fetch or to the mock.

It's still early days for us, but we're going to have to rewrite all our unit tests to this new setup away from module mocking cross-fetch.

Once we've something working, I kinda want to take it to WinterCG to propose a standardised way to mock fetch.

Interestingly, undici, which powers node's native fetch, does have built-in mocking https://undici.nodejs.org/#/docs/api/MockAgent

I can't see any way to use this with fetch, but I think we could just use undici directly instead of the fetch wrapper

oh and here's a useful reference from another project making the same conversion from nock to MockAgent: https://github.com/vansergen/poloniex-node-api/pull/91

ThisIsMissEm commented 1 year ago

Oh? Interesting! I thought it worked for that. Either way, we'll hopefully have a new module out soon that'll help