Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix getContextNameFromHash edge case not returning the expected value #392

Open
aurelticot opened this issue Dec 29, 2023 · 1 comment
Open
Labels
bug Something isn't working

Comments

@aurelticot
Copy link
Member

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 aurelticot added the bug Something isn't working label Dec 29, 2023
@aurelticot
Copy link
Member Author

@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:

const endpoints: ServiceEndpoint[] = <ServiceEndpoint[]> service.serviceEndpoint
for (let e in endpoints) {
let endpointUri = endpoints[e]
endpointUri = endpointUri.substring(0, endpointUri.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') {
return response.data.result.contextName
}
} catch (err) {
// ignore errors, try another endpoint
}
}
throw new Error(`Unable to access any endpoints associated with context hash ${contextHash}`)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant