activitypub-express icon indicating copy to clipboard operation
activitypub-express copied to clipboard

request-promise-native breaks promises in Jest

Open gregid opened this issue 5 years ago • 7 comments

Related: #12

As it is just by importing request-promise-native or ActivitypubExpress that uses it, the async tests in Jest will fail in some instances. Here is an example (uncomment any requires: ActivitypubExpress or request to see the errors):

const jsonld = require('jsonld')
// const ActivitypubExpress = require('activitypub-express')
// const request = require('request-promise-native')

const context = [
  'https://www.w3.org/ns/activitystreams',
  'https://w3id.org/security/v1'
]
const obj = { 
  '@id': 'https://example.org/personID',
  '@type': 'https://www.w3.org/ns/activitystreams#Person',
  'https://w3id.org/security#publicKey': {
    '@id': 'https://example.org/personID#main-key',
    'https://w3id.org/security#owner': 'https://example.org/personID'
  }
}
const opts = {
  compactArrays: false,
  documentLoader: jsonld.documentLoaders.node()
}

it('jsonld.compact activitystreams+security [promise]', function () {
  expect.assertions(1);

  return jsonld.compact(obj, context, opts).then((result) => {
    // console.log('result:', JSON.stringify(result, null, ' '))
    expect(result).toStrictEqual({
      '@context': [
        'https://www.w3.org/ns/activitystreams',
        'https://w3id.org/security/v1'
      ],
      '@graph': [
        {
          id: 'https://example.org/personID',
          type: 'Person',
          publicKey: [
            {
              id: "https://example.org/personID#main-key",
              "sec:owner": [ "https://example.org/personID",],
            },
          ],
        }
      ]
    })
  }).catch((error) => {
    console.error(error)
    throw error
  })
})

In this instance it will fail on fetching https://w3id.org/security/v1 since it will first return Status Code 302 (document moved). The proper promise with @context will never resolve breaking this and messing up subsequent tests.

To install Jest: npm install jest, to run single test file: npx jest -- testpath/compact.spec.js

Besides making it impossible to test projects using ActivitypubExpress with Jest I am concerned that this may also indicate other promise related issues in the future.

I was thinking node-fetch might be a good fit although not complete: https://github.com/node-fetch/node-fetch/pull/480

gregid avatar Apr 16 '21 01:04 gregid

Wow that's wild. I found this package was created to fix this issue with jest - does that work for you?

I would love to migrate away from request, but the http-signatures require low-level access to request headers at a point in between setting the content and sending the request, and I haven't yet found another library where this is possible

wmurphyrd avatar Apr 16 '21 02:04 wmurphyrd

I wasn't able to make it work yet, but will see tomorrow as I am done for today. In the PR I linked above the use case is exactly the same, but it doesn't look like it will go upstream any time soon. Still, the http signatures seem like such a reasonable thing that these libraries should implement... surprising there are none that bother.

gregid avatar Apr 16 '21 02:04 gregid

I had no success with the package but it points to the root of the problem - both activitypub-express and jsonld use request library which is causing the problem here.

I've noticed jsonld version 5.x moves away from request to their own @digitalbazaar/http-client unfortunately this new jsonld version causes another problem for activitypub-express: digitalbazaar/jsonld.js#451

gregid avatar Apr 16 '21 10:04 gregid

Maybe the solution is to move away from http-signature. I'm already having to run a patched version, maybe I should just make my own that is more flexible and can work with other request libraries

wmurphyrd avatar Apr 16 '21 14:04 wmurphyrd

I went on a little library hunt in search for request replacement and I think I may have found one that should fit the bill:

https://github.com/simov/request-compose https://dev.to/simov/composable-http-client-for-nodejs-83f

What do you think?

gregid avatar Apr 16 '21 14:04 gregid

Also there is http-signature-header by digitalbazaar as well:

https://www.npmjs.com/package/http-signature-header https://github.com/digitalbazaar/http-signature-header

gregid avatar Apr 16 '21 15:04 gregid

With #41 activitypub-express can be successfully tested with jest

gregid avatar Apr 19 '21 20:04 gregid