Skip to content

Conversation

@Devnil434
Copy link

@Devnil434 Devnil434 commented Nov 13, 2025

Description

This PR fixes the issues in the netlify/functions/github_discussions.ts function related to error handling, input validation, and GraphQL query security.

✅ Fixes Applied:

  • Fixed the Error Response Format Bug:

    • Replaced the incorrect message property in the return statement with a properly formatted body property containing JSON stringified content.
    • Ensures full compliance with Netlify Function response structure.
  • Fixed GraphQL Injection Vulnerability:

    • Removed string interpolation in the GraphQL query.
    • Implemented proper query variables to prevent potential GraphQL injection attacks.
    • Enhanced security and reliability of the GitHub Discussions integration.
  • Added Input Validation:

    • Implemented robust JSON parsing with error handling.
    • Added validation for required fields such as title and feedback.
    • Returns proper HTTP error codes (400, 422, 500) based on validation or runtime errors.
  • Improved Error Handling:

    • Added explicit handling for JSON parsing errors and missing parameters.
    • Improved error messages for better debugging and API transparency.
    • Ensured all responses conform to Netlify's expected { statusCode, body } structure.

Result

  • The function now returns valid, structured responses on both success and failure.
  • Prevents crashes caused by missing or malformed responses.
  • Significantly improves code robustness, error traceability, and API security.

Related issue(s)
Fixes #4577


Example Test Results

Scenario Expected Result Status
Invalid GitHub token Returns HTTP 401 with error message
Missing discussion title Returns HTTP 400 with validation error
Successful creation Returns HTTP 200 with discussion data
Network error Returns HTTP 500 with detailed message
Screenshot 2025-11-13 165245 Screenshot 2025-11-13 165236 Screenshot 2025-11-13 165223 Screenshot 2025-11-13 165212 Screenshot 2025-11-13 165152 Screenshot 2025-11-13 165124

Notes

  • All tests were run using local Netlify CLI (netlify dev) and verified in both success and error states.
  • Function is now aligned with Netlify best practices and AsyncAPI website API standards.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved request validation and error handling with clearer, more descriptive error messages.
    • Added validation to ensure all required submission information is provided.
    • Enhanced error responses to provide more useful feedback when issues occur.
    • Strengthened robustness of data processing to handle edge cases better.

@netlify
Copy link

netlify bot commented Nov 13, 2025

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1dd6bea
🔍 Latest deploy log https://app.netlify.com/projects/asyncapi-website/deploys/6919e3a60f384f0008a4c3b4
😎 Deploy Preview https://deploy-preview-4582--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Nov 13, 2025

We require all PRs to follow Conventional Commits specification.
More details 👇🏼

 No release type found in pull request title "fix github_discussions.tsx file bug". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 13, 2025

Walkthrough

This update enhances the GitHub Discussions Netlify function with robust error handling, field validation, and parameterized GraphQL mutations. Error responses are now properly wrapped in a JSON body property to comply with Netlify Function response structure, addressing a bug where errors were malformed.

Changes

Cohort / File(s) Change Summary
Error Handling & GraphQL Improvements
netlify/functions/github_discussions.ts
Added try/catch JSON parsing with {} defaults; added validation for required title and feedback fields returning 400 on failure; converted GraphQL mutation from string interpolation to parameterized variables ($repositoryId, $categoryId, $title, $body); fixed error response formatting by wrapping messages in JSON body property to comply with Netlify Function response structure

Sequence Diagram

sequenceDiagram
    participant Client
    participant Function as github_discussions Function
    participant GraphQL as GitHub GraphQL API

    Client->>Function: POST with event body

    rect rgb(200, 220, 255)
    Note over Function: Parse & Validate
    Function->>Function: try/catch JSON.parse(event.body)
    alt Parse succeeds
        Function->>Function: Validate title & feedback present
        alt Validation fails
            Function-->>Client: 400 {body: {message: "Missing required fields"}}
        end
    else Parse fails
        Function-->>Client: 400 {body: {message: "Invalid JSON"}}
    end
    end

    rect rgb(200, 255, 220)
    Note over Function,GraphQL: GraphQL Mutation (Parameterized)
    Function->>GraphQL: Mutation with variables<br/>($repositoryId, $categoryId, $title, $body)
    alt API succeeds
        GraphQL-->>Function: Discussion created
        Function-->>Client: 200 {body: {url, message}}
    else API fails
        GraphQL-->>Function: Error response
        Function-->>Client: 500 {body: {message: "API error"}}
    end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Validation logic: Review the new field validation rules and 400 response handling to ensure completeness
  • GraphQL parameterization: Verify the variable mapping and mutation structure correctly aligns with GitHub API expectations
  • Error response formatting: Confirm the JSON.stringify wrapping matches Netlify Function response requirements and frontend expectations

Suggested labels

ready-to-merge

Suggested reviewers

  • derberg
  • akshatnema
  • sambhavgupta0705
  • anshgoyalevil
  • asyncapi-bot-eve
  • Mayaleeeee

Poem

🐰 With try-catch blocks and validation keen,
GraphQL variables keep queries clean,
Error responses now properly wrapped,
No more malformed responses getting trapped!
The GitHub discussions flow runs true,
Fixed at last—this rabbit's debut! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix github_discussions.tsx file bug' is related to the changeset but is vague and generic, lacking the Conventional Commits prefix (e.g., 'fix:') required by the project. Update the title to follow Conventional Commits format: 'fix: github_discussions.ts error handling and GraphQL injection prevention' to be more specific and compliant with project standards.
Out of Scope Changes check ❓ Inconclusive The pull request includes additional improvements beyond issue #4577 requirements: robust JSON parsing with try/catch, input validation for title/feedback fields, and GraphQL query parameterization to prevent injection attacks. Clarify whether the additional security improvements (GraphQL parameterization, input validation, JSON parsing robustness) are intentional enhancements or should be separated into a distinct issue for tracking.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The pull request successfully addresses all coding requirements from issue #4577: proper error response formatting with body property, JSON stringification, and appropriate HTTP status codes for various error scenarios.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cba652 and 6f2f8aa.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • netlify/functions/github_discussions.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: asyncapi-bot
Repo: asyncapi/website PR: 0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
🧬 Code graph analysis (1)
netlify/functions/github_discussions.ts (1)
netlify/functions/save-discussion-background/Reposity.ts (1)
  • createDiscussion (56-87)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Redirect rules - asyncapi-website
  • GitHub Check: Header rules - asyncapi-website
  • GitHub Check: Pages changed - asyncapi-website

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Nov 13, 2025

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 40
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-4582--asyncapi-website.netlify.app/

@Devnil434 Devnil434 closed this Nov 16, 2025
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 this pull request may close these issues.

[BUG] Improper Error Handling Response in Github Discussions

2 participants