Error reporting for protocol definition missing protocolPath could be more descriptive

[frankhinek]

Task Details:

Steps to Reproduce

1. Instantiate a DWN (e.g., in browser or Node env).

2. Create DIDs/keys for Alice and Bob

3. ProtocolsConfigure.create() and process the following definition for both DWNs:

     const protocol = 'chat';
     const protocolDefinition = {
     "recordDefinitions": [
       {
         "id": "message",
         "schema": "chat/message"
       }
     ],
     "records": {
       "message": {
         "allow": [
           {
             "actor": "anyone",
             "actions": ["write"]
           },
           {
             "actor": "author",
             "actions": ["read"]
           },
           {
             "actor": "recipient",
             "actions": ["read"]
           }
         ]
       }
     }
   };

4. Create a RecordsWrite message signed by Alice that is written to Bob's DWN referencing the chat protocol and chat/message schema. Excerpt from RecordsWrite.create():

     protocol: 'chat',
     protocolPath: 'message',
     schema: 'chat/message'

5. When you call processMessage the response returned is:

    {
      status: {
        code: 401,
        detail: "Cannot read properties of undefined (reading 'split')",
      },
      entries: undefined,
      data: undefined,
    }

Expected Behavior

I made a mistake in the protocol definition by failing to include a protocolPath for both of the allow rules that involve the author and recipient actors. It should have been:

  const protocolDefinition = {
  "recordDefinitions": [
    {
      "id": "message",
      "schema": "chat/message"
    }
  ],
  "records": {
    "message": {
      "allow": [
        {
          "actor": "anyone",
          "actions": ["write"]
        },
        {
          "actor": "author",
          "protocolPath": "message",
          "actions": ["read"]
        },
        {
          "actor": "recipient",
          "protocolPath": "message",
          "actions": ["read"]
        }
      ]
    }
  }
};
```

Given that, I would expect an error to occur. However, it would have saved a lot of troubleshooting time if the error had been more descriptive than Cannot read properties of undefined (reading 'split'). Fortunately, there aren't that many uses of .split() in the code base so it didn't take all that long to track down.

Potential Improvements

Idea A

One potential improvement would be to detect the missing protocolPath in the Allow Rules in ProtocolAuthorization.getMessage():

https://github.com/TBD54566975/dwn-sdk-js/blob/f8a150e4e5a276d521fada8e4a6f0bc0c6d9e3c5/src/core/protocol-authorization.ts#L384-L388

and throw a more informative message such as 'protocolPath' missing for 'allow' rule.

Idea B

Related to Issue #332, perhaps this could be caught during ProtocolsConfigure message ingestion?

Picking Up This Issue:

• If you'd like to work on this, please comment "picking this up" below, and I'll assign the issue to you

Questions:

• If you need assistance or clarification, feel free to comment below.
• Feeling overwhelmed? Join our #hack-together channel and collaborate with a buddy.
• New to open-source? Check out our Hacktober #getting-started channel.
• Leaderboard: Check your ranking after finishing this task.

Resources:

• Creating a Pull Request: If you're new to GitHub and unsure how to create a pull request, follow this step-by-step guide.

Remember, communication is key! If you have any questions or face any challenges, we're here to help so please don't hesitate to reach out.

Good Luck! 🍁

[Rizina]

picking this up

[EbonyLouis]

@Rizina Assigned 🥳 reach out if you need any help

[Rizina]

@EbonyLouis it seems the original file mentioned changed a lot since the issue was opened.
Can you please guide me what exactly should I do now 🙏

[EbonyLouis]

@Rizina I think the issue comment was updated with more information on how to tackle this issue. Let me know if it's still confusing to you in any way

[flothjl]

@EbonyLouis is this available?

[flothjl]

@frankhinek @diehuxx Is this still an issue? Better error messaging for of was added in #520.

[diehuxx]

@flothjl @EbonyLouis No longer an issue, #520 did in fact resolve this.