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

Problem Report not delivered until connection is active #2650

Closed
usingtechnology opened this issue Dec 4, 2023 · 4 comments · Fixed by #2653
Closed

Problem Report not delivered until connection is active #2650

usingtechnology opened this issue Dec 4, 2023 · 4 comments · Fixed by #2653
Assignees

Comments

@usingtechnology
Copy link
Contributor

See #2599 and PR #2600.

Problem Reports require an active connection for delivery. However, if the issue is in the early stages of the connection/didexchange protocols, we have no way of successfully communicating that back to the other agent. We do have logic that catches exceptions and fires a ProblemReport in the request and response handlers but those reports will be swallowed and not delivered because the underlying connection is not active.

SKIP_ACTIVE_CONN_CHECK_MSG_TYPES = [
    "didexchange/1.0/request",
    "didexchange/1.0/response",
    "connections/1.0/invitation",
    "connections/1.0/request",
    "connections/1.0/response",
]

....

        if (
            message.connection_id
            and msg_type
            and msg_type not in SKIP_ACTIVE_CONN_CHECK_MSG_TYPES
            and not await super().conn_rec_active_state_check(
                profile=context.profile,
                connection_id=message.connection_id,
            )
        ):
            raise RuntimeError(
                f"Connection {message.connection_id} is not ready"
                " which is required for sending outbound"
                f" message {msg_id} of type {msg_type}."
            )
        return await self._send(context.profile, message, self._inbound_message)

Should we add problem report the skip types? Which would allow the message to be delivered? or add some other flag/logic that would allow problem reports in the skipped type protocol stages to be delivered? Like a problem report that occurs during the execution of a skipped type?

@swcurran
Copy link
Contributor

swcurran commented Dec 4, 2023

MY a bit confused by this.

I don’t think an “active” connection is needed to send a problem report — or at least I don’t think one should not be needed. The minimum needed to send a DIDComm message is the DIDComm Messaging Service Endpoint of the other party, which is a lower bar than an active connection.

If I get a message from another party that is bad in the process of establishing a connection, and I have the DIDComm Messaging Service Endpoint for that party, I think I should be able to send them a problem report. Since each of the listed “skip” message types contain a DIDComm Messaging Service Endpoint, if they are bad, I should be able to send a Problem Report back.

Likewise, if I get a message with no known context (perhaps a connection request that I have abandoned) and there is a DIDComm Messaging Service Endpoint in it, I can (send a Problem Report).

@usingtechnology
Copy link
Contributor Author

Yes, we absolutely should be able to send the report but the current implementation prevents it. Because the message is of type notification/1.0/problem-report the current implementation will not deliver it until the connection has become active (for our purposes response state is "active" enough).

The errors that become problem reports are caught, turned into problem reports then the responder sends them, when they hit the dispatcher - that's where it checks the message type before it sends the DIDComm message. So we could add"notification/1.0/problem-report to the skip message types or use some other mechanism to allow those to be delivered.

@usingtechnology
Copy link
Contributor Author

Sorry, it's: didexchange/1.0/problem_report and connections/1.0/problem_report. Was doing a little investigation and it is more than just adding those to the SKIP_ACTIVE_CONN_CHECK_MSG_TYPES. The actual handler (when receiving a problem report) needs to find the connection so it can set as abandoned. Need to figure out how to resolve given limited information in the message. Looking into the resolution logic from other handlers.

@usingtechnology usingtechnology self-assigned this Dec 5, 2023
@usingtechnology
Copy link
Contributor Author

Good progress on this, need to include #2598 with this; just makes sense to complete both connection and did problem report fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants