verida / verida-js

The Verida SDK provides several SDKs to interact with the Verida Network
https://developers.verida.network
ISC License
1.31k stars 37 forks source link

Fix `getContextNameFromHash` edge case not returning the expected value #392

Open aurelticot opened 11 months ago

aurelticot commented 11 months ago

The getContextNameFromHash methods iterates over the endpoints and requests the contextName for a contextHash.

It only returns a value if one endpoint replies successfully. If they all fail (it is not caught as each individual error is intentionally ignored assuming another endpoint would succeed), getContextNameFromHash will return void/undefined to the consumer while the returned typescript type is string. In such case where there is no value, an error should be thrown.

Suggestion is, instead of iterating on the endpoints, we prepare an array of promises making the calls on the endpoints, each promise properly resolves with the value or rejects if there is no value. Then we execute the promises with Promise.any, it will resolve as soon as a first promise resolves successfully. If they all fail, it will fail. From there, we can return the value or throw an error.

It will also improve the performance as the calls to the endpoints will be in parallels

aurelticot commented 11 months ago

@tahpot for this bug, I haven't tested or pushed any commit but here's my suggestion:

Note: Promise.any is es2021 so you'll have to update the tsconfig

  public async getContextNameFromHash(contextHash: string, didDocument?: DIDDocument) {

    // ... 

    const endpoints: ServiceEndpoint[] = <ServiceEndpoint[]>service.serviceEndpoint

    if (endpoints.length === 0) {
      throw new Error(`Unable to access any endpoints associated with context hash ${contextHash}`)
    }

    try {
      return Promise.any(endpoints.map(async (endpoint) => {
        const endpointUri = endpoint.substring(0, endpoint.length-1)  // strip trailing slash

        const consentMessage = `Obtain context hash (${contextHash}) for server: "${endpointUri}"?\n\n${did}\n${timestamp}`
        const signature = await this.account!.sign(consentMessage)

        try {
            const response = await Axios.post(`${endpointUri}/user/contextHash`, {
            did,
            timestamp,
            signature,
            contextHash
          });

          if (response.data.status !== 'success') {
            throw new Error(`Endpoint ${endpointUri} failed to return context name associated with context hash ${contextHash}`)
          }

          return response.data.result.contextName as string // TODO: Ideally should validate before returning to ensure the expected value type
        } catch (error) {
          throw new Error(`Endpoint ${endpointUri} failed to return context name associated with context hash ${contextHash}`)
        }
      }))
    } catch (error) {
      throw new Error(`All endpoints failed to locate context name associated with context hash ${contextHash}`)
    }
  }

Located here: https://github.com/verida/verida-js/blob/b4e097271efcf472aa5530190ca7bc5addbcce41/packages/client-ts/src/client.ts#L440-L464