Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Allow all opts for cmd:cat, to support the timeout flag #1034

Closed

Conversation

cle1000
Copy link

@cle1000 cle1000 commented Jun 27, 2019

I need the timeout flag for my application, but I recognized that js-ipfs-http-client does not allow it for the command cat. I modified that file such that it forwards the opts as it is done by the get command.

With this call it would be possible to set the a timeout for the cat command.

ipfsClient.cat(`/ipfs/${YOUR_HASH}`,{timeout: '10s'}, callback)

@cle1000 cle1000 marked this pull request as ready for review June 27, 2019 07:53
@alanshaw
Copy link
Contributor

Is timeout a valid option? It's not listed here: https://docs.ipfs.io/reference/api/http/#api-v0-cat

@cle1000
Copy link
Author

cle1000 commented Jul 10, 2019

Hi,
i use the docker image: jbenet/go-ipfs:latest and the timeout flag is working. In this pull request I did not add the timeout flag explicit, I changed the code for the options to allow all opts like for the command get.

I also test it with the docker image listed above, and the timeout flag works.

@alanshaw
Copy link
Contributor

Thanks for this @cle1000, would you be able to add a test here https://github.com/ipfs/interface-js-ipfs-core/blob/master/src/files-regular/cat.js to ensure it's working as expected?

@cle1000
Copy link
Author

cle1000 commented Jul 13, 2019

Hi @alanshaw, I already created the test method, but I can not test it. I found out that this repro is included in the js-ipfs-http-client, but I am not able to run the tests in this repro. This is the output when I run npm test:

> aegir test

Test Node.js


  .swarm.peers
    ✓ handles a peer response
    ✓ handles a go-ipfs <= 0.4.4 peer response
    ✓ handles an ip6 quic peer
    ✓ handles unvalidatable peer addr
    ✓ handles an error response

  .commands
    1) "before all" hook for "lists commands"

  ipfs-http-client constructor tests
    parameter permuations
      ✓ none
      ✓ opts
      ✓ multiaddr dns4 string (implicit http)
      ✓ multiaddr dns4 string (explicit https)
      ✓ multiaddr dns4 string, explicit https in opts
      ✓ multiaddr ipv4 string (implicit http)
      ✓ multiaddr ipv4 string (explicit https)
      ✓ multiaddr instance
      ✓ host and port strings
      ✓ host, port and api path
      ✓ throws on invalid multiaddr
    integration
      2) "before all" hook for "can connect to an ipfs http api"

  custom headers
    3) "before all" hook for "are supported"
    4) "after all" hook for "are supported"

  .dag
    5) "before all" hook for "should be able to put and get a DAG node with format dag-pb"

  .diag
    6) "before all" hook in ".diag"

  .getEndpointConfig
    7) "before all" hook for "should return the endpoint configuration"

  exports
    ✓ should export the expected types and utilities

  .files (the MFS API part)
    8) "before all" hook for ".add file for testing"

  .get (specific go-ipfs features)
    9) "before all" hook for "no compression args"

  interface-ipfs-core tests
    .bitswap.stat
      10) "before all" hook for "should get bitswap stats"
    .bitswap.wantlist
      11) "before all" hook for "should get the wantlist"
    .block.put
      12) "before all" hook for "should put a buffer, using defaults"
    .block.get
      13) "before all" hook for "should get by CID object"
    .block.stat
      14) "before all" hook for "should stat by CID"
    .bootstrap.add
      15) "before all" hook for "should return an error when called with an invalid arg"
    .bootstrap.list
      16) "before all" hook for "should return a list of peers"
    .bootstrap.rm
      17) "before all" hook for "should return an error when called with an invalid arg"
    .config.get
      18) "before all" hook for "should retrieve the whole config"
    .config.set
      19) "before all" hook for "should set a new key"
    .config.replace (FIXME Waiting for fix on go-ipfs https://github.com/ipfs/js-ipfs-http-client/pull/307#discussion_r69281789 and https://github.com/ipfs/go-ipfs/issues/2927)
      - should replace the whole config
      - should replace to empty config
    .dag.get
      20) "before all" hook for "should get a dag-pb node"
    .dag.put
      21) "before all" hook for "should put dag-pb with default hash func (sha2-256)"
^C

Do you have any suggestions to remove this ? I think I the error comes from an unavailable ipfs node, but I dont know how to resolve it.

So when the tests in the js-ipfs-http-client repros are working. I would link the interface-js-ipfs-core locally to my local js-ipfs-http-client and check if the test is working as expected.

This would be my test method:

    it('should fail after a timeout of 2 seconds', (done) => {
      const timeout = 2;
      const startTime = new Date();

      ipfs.cat(`/ipfs/QmJJJJJJJJJJJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP`, {
        timeout : `${timeout}s`
      }, (err, data) => {
          let timeForRequest = (new Date () - startTime)/1000;
          expect(data).to.not.exist()
          expect(timeForRequest).to.not.greaterThan(timeout);
          expect(timeForRequest).to.not.lessThan(timeout);
          done()
      })
    })

@cle1000
Copy link
Author

cle1000 commented Aug 19, 2019

Thanks for this @cle1000, would you be able to add a test here https://github.com/ipfs/interface-js-ipfs-core/blob/master/src/files-regular/cat.js to ensure it's working as expected?

@alanshaw Can you give me a hint, how i should proceed?

@alanshaw
Copy link
Contributor

alanshaw commented Nov 5, 2019

Closing as this is now supported for cat. Documentation for it will be added with #1103

@alanshaw alanshaw closed this Nov 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants