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

Improve error reporting when action fail to parse correctly the payload #359

Open
4 of 10 tasks
cedric-appdirect opened this issue Nov 21, 2024 · 1 comment
Open
4 of 10 tasks
Labels
auto-triage-skip Prevent this issue from being closed due to lack of activity enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@cedric-appdirect
Copy link

Description

In case of a payload parse error, we get the message Invalid input! Failed to parse contents of the provided payload. which is not very specific. It would be great to have a better message that highlight which line is the source of the error.

What type of issue is this? (place an x in one of the [ ])

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • example code related
  • testing related
  • discussion

Requirements (place an x in each of the [ ])

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

When the payload is malformed, it result in a parse error message which does not guide us on where to look for to find the problem. It would be great if we could at least have the line where the first error happen.

Reproducible in:

package version: latest

node version:

OS version(s):

Steps to reproduce:

  1. Send a malformed payload
  2. Figure out why it is malformed

Expected result:

Display the line of the first error found. I understand there is a difficulty with the optimistic detection of the file format (yaml vs json), but it is likely that a yaml file parse with less error when there is a malformed payload with the yaml parser than with the json parser.

Actual result:

Just say: Invalid input! Failed to parse contents of the provided payload.

@vegeris vegeris added the enhancement New feature or request label Nov 21, 2024
@vegeris vegeris added this to the 2.x milestone Nov 21, 2024
@vegeris vegeris added the auto-triage-skip Prevent this issue from being closed due to lack of activity label Nov 21, 2024
@zimeg
Copy link
Member

zimeg commented Nov 23, 2024

Hey @cedric-appdirect 👋 Thanks for raising this issue! I agree we can improve this erroring a bit 🙏

To find the payload that's causing these parsing errors, I'll recommend rerunning the failed job with debug logging enabled. This includes the input payload before parsing happens, and a YAML or JSON validator can be used to find edges.

That's not a fix, but might be helpful as this continues to bring confusion. Below are more details towards finding a fix.


Right now YAML parsing errors are included in debug logs, but this requires a rerun to view AFAIK. An attempt of including the caught JSON error cause is included when thrown but I'm noticing that this isn't included in the final output:

file:///home/runner/work/_actions/slackapi/slack-github-action/v2.0.0/src/content.js:93
      throw new SlackError(
^
SlackError: Invalid input! Failed to parse contents of the provided payload
    at Content.getContentPayload (file:///home/runner/work/_actions/slackapi/slack-github-action/v2.0.0/src/content.js:93:1)
    at Content.get (file:///home/runner/work/_actions/slackapi/slack-github-action/v2.0.0/src/content.js:28:1)
    at new Config (file:///home/runner/work/_actions/slackapi/slack-github-action/v2.0.0/src/config.js:115:1)
    at send (file:///home/runner/work/_actions/slackapi/slack-github-action/v2.0.0/src/send.js:13:1)
    at file:///home/runner/work/_actions/slackapi/slack-github-action/v2.0.0/src/index.js:9:1
    at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
    at ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:1[23](https://github.com/zimeg/pr_targets/actions/runs/11981961769/job/33409143674#step:2:24):5)

The YAML parsing errors happening before the above error (which was caused by failed JSON parsing) might match:

##[debug]Failed to parse input payload as YAML
##[debug]missed comma between flow collection entries (3:5)

Display the line of the first error found ... it is likely that a yaml file parse with less error when there is a malformed payload with the yaml parser than with the json parser.

We should investigate the reason of the disappearing cause since I believe that can add a lot of clarity to errors:

SlackError: Invalid input! Failed to parse contents of the provided payload
    at Content.getContentPayload (file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/content.js:100:1)
    at Content.get (file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/content.js:28:1)
    ... 5 lines matching cause stack trace ...
    at asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5) {
  [cause]: SyntaxError: Expected property name or '}' in JSON at position 2
      at JSON.parse (<anonymous>)
      at Content.getContentPayload (file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/content.js:98:1)
      at Content.get (file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/content.js:28:1)
      at new Config (file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/config.js:115:1)
      at send (file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/send.js:13:1)
      at file:///Users/eden.zimbelman/programming/tools/slack-github-action/src/index.js:9:1
      at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
      at ModuleLoader.import (node:internal/modules/esm/loader:316:24)
      at asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:123:5)
}

I'm so interested in how the "right" error can be shown here too! To share more context, YAML was introduced for the convenience of writing workflows in the syntax of GitHub Actions, but the Slack API methods best support JSON.

FWIW falling back to the JSON errors feels right to me with the API considerations, but I understand that's not so helpful when it's YAML parsing that raises errors.

You point out well that we might be somewhat limited with the optimistic filetype detection... This might require separate explorations to figure out how to return the error cause, then also returning the relevant parsing error.

I'll mark this as a "good first issue" and encourage additional thoughts or PRs, but this is something we'll look into for an upcoming release. Thanks again for calling this out! 🚀

@zimeg zimeg added the good first issue Good for newcomers label Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-triage-skip Prevent this issue from being closed due to lack of activity enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants