hyperledger / identus-edge-agent-sdk-ts

Apache License 2.0
24 stars 13 forks source link

Issues handling didcomm message errors #308

Closed amagyar-iohk closed 2 weeks ago

amagyar-iohk commented 1 month ago

Is this a regression?

Yes

Description

Brief

When didcomm message returns a http status code other than ok the message becomes undefined due a try-catch block. This issue's goal is to enable an error management for didcomm errors. Spec: https://identity.foundation/didcomm-messaging/spec/#problem-reports

FetchApi.ts

  async request<T>(
    method: HttpMethod,
    urlStr: string,
    urlParameters: Map<string, string> = new Map(),
    httpHeaders: Map<string, string> = new Map(),
    body?: string | Record<string, any>
  ): Promise<ApiResponse<T>> {
    // ...
    if (response.ok) {
      return new ApiResponse(
        data,
        response.status,
        response.statusText,
        response.headers
      );
    }
    throw new ApiError(response.status, response.statusText, data);
}

Mercury.ts

  async sendMessageParseMessage(
    message: Domain.Message
  ): Promise<Domain.Message | undefined> {
    const responseBody = await this.sendMessage<any>(message);
    try {
      const responseJSON = JSON.stringify(responseBody);
      return await this.unpackMessage(responseJSON);
    } catch (err) {
      return undefined
    }
  }

Any ApiError thrown will cause message to return undefined as per described in the Mercury.ts#sendMessageParseMessage method

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in

No response

Anything else?

No response

elribonazo commented 1 month ago

@amagyar-iohk has the agent been fixed? Because the agent responds no valid JSON objects when some exceptions occur.

If you could validate this it would be great.

If the Agent does not respond with a valid json object then, we could set a default JSON object or error instead of undefined. But the point is that the agent should ALWAYS return valid json objects, no matter what happened, mainly because we are requesting appliication/json objects in the header... but its returning something diff

amagyar-iohk commented 1 month ago

Yes, this is the api response for the scenario which the piuri is wrong

ApiResponse {
  headers: Headers {
    'content-length': '271',
    'content-type': 'application/json',
    date: 'Tue, 22 Oct 2024 12:12:55 GMT'
  },
  body: {
    status: 422,
    type: 'error:DIDCommControllerError:UnsupportedPIURI',
    title: 'Unsupported P I U R I',
    detail: 'The Protocol Identifier URI (URI) found in the DIDComm message is not supported: PIURI=potato',
    instance: 'error:instance:ae85af1e-53ee-41ae-9afc-63f05bb17556'
  },
  status: 422,
  statusText: 'Unprocessable Entity'
}
elribonazo commented 2 weeks ago

@amagyar-iohk what shall we do with this?

amagyar-iohk commented 2 weeks ago

This is going to be closed as we didn't implement the didcomm error report in agent yet