chore: remove response text check from hooks middleware for output ho…

[VisargD]

…oks and let individual plugin handle it

Description

All the individual plugin should make the content checks based on their requirements. All the existing plugins already check if the text exists or not.

Having this check causes issues where user might want to have a webhook plugin executed at the end but the response content is empty. Example: Anthropic returns empty content ("") for some responses with tool calls assistant message.

Example:

{
    "id": "<>",
    "object": "chat.completion.chunk",
    "created": <>,
    "choices": [
      {
        "index": 0,
        "finish_reason": "tool_use",
        "message": {
          "role": "assistant",
          "content": "",
          "tool_calls": [
            {
              "id": "<>",
              "type": "function",
              "function": {
                "name": "sample_name",
                "arguments": "sample_arguments"
              }
            }
          ]
        }
      }
    ],
    "model": "claude-sonnet-4-20250514",
    "usage": {
      "completion_tokens": 1,
      "cache_read_input_tokens": 0,
      "cache_creation_input_tokens": 1,
      "prompt_tokens": 1
      "total_tokens": 12
    }
  }

Motivation

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)

How Has This Been Tested?

• Unit Tests
• Integration Tests
• Manual Testing

Screenshots (if applicable)

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Related Issues

[matter-code-review]

Summary By MatterAI

🔄 What Changed

Removed the response text check from hooks middleware for output hooks. Specifically, the condition that checked if the response status code is not 200 was removed from the hooks middleware logic.

🔍 Impact of the Change

This change simplifies the hook execution logic by removing an unnecessary condition that was preventing certain hooks from executing. By removing the status code check, hooks with eventType 'afterRequestHook' will now execute regardless of the response status code, which provides more consistent behavior.

📁 Total Files Changed

• src/middlewares/hooks/index.ts: Removed response status code check condition from hook execution logic.

🧪 Test Added

N/A - No tests were added in this PR.

🔒 Security Vulnerabilities

N/A - No security vulnerabilities were identified in this change.

Motivation

To simplify hook execution logic and ensure consistent behavior for afterRequestHooks regardless of response status code.

Type of Change

• Refactoring (no functional changes)

How Has This Been Tested?

• Unit Tests
• Integration Tests
• Manual Testing

Screenshots (if applicable)

N/A

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

Related Issues

N/A

Tip

Quality Recommendations

1. Consider adding a comment explaining why the status code check was removed to provide context for future developers

2. Add unit tests to verify that hooks with eventType 'afterRequestHook' now execute regardless of response status code

Tanka Poem ♫

Code simplified now,
Hooks flow without status checks,
Responses set free.
Middleware logic refined—
✨ Elegance in fewer lines.

Sequence Diagram

sequenceDiagram
    participant Client
    participant HooksMiddleware
    participant Hook
    participant Context
    
    Client->>HooksMiddleware: Request
    HooksMiddleware->>HooksMiddleware: shouldSkipHook(hook, context)
    Note over HooksMiddleware: Previous logic included status code check
    Note over HooksMiddleware: (hook.eventType === 'afterRequestHook' && context.response.statusCode !== 200)
    Note over HooksMiddleware: This condition has been removed
    
    alt Hook should not be skipped
        HooksMiddleware->>Hook: Execute hook
        Hook->>Context: Modify context if needed
    else Hook should be skipped
        HooksMiddleware-->>HooksMiddleware: Skip hook execution
    end
    
    HooksMiddleware->>Client: Response

Loading

[matter-code-review]

This PR removes a condition that was preventing hooks with eventType 'afterRequestHook' from executing when the response status code wasn't 200. This is a good simplification that will make hook behavior more consistent. I have one minor suggestion below.