Skip to content

add support for event stream response in api executor #2686

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

Merged
merged 2 commits into from
Jun 18, 2025

Conversation

notshivansh
Copy link
Contributor

No description provided.

@notshivansh notshivansh requested a review from Copilot June 17, 2025 14:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for handling text/event-stream responses in the API executor and expands logging in several utility classes.

  • Introduce event-stream response handling with timeout in ApiExecutor
  • Refactor header extraction and default body fallback in ApiExecutor
  • Enhance logging around generated query sets and operations in Filter and Executor
  • Add new header constants in HttpRequestResponseUtils

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
libs/utils/src/main/java/com/akto/testing/ApiExecutor.java Add event-stream support, refactor headers/body handling, adjust logging calls
libs/utils/src/main/java/com/akto/test_editor/filter/Filter.java Initialize and log generated query sets
libs/dao/src/main/java/com/akto/util/HttpRequestResponseUtils.java Define new constants for event-stream and content-type headers
apps/testing/src/main/java/com/akto/test_editor/execution/Executor.java Add logging for AI-generated operations and null-check fix
Comments suppressed due to low confidence (2)

libs/utils/src/main/java/com/akto/testing/ApiExecutor.java:49

  • Removing the LogDb parameter from infoAndAddToDb may prevent logs from being classified and persisted correctly. Consider restoring the LogDb.TESTING argument or updating the logger implementation to handle single-argument calls.
                        loggerMaker.infoAndAddToDb("Rate limit hit, sleeping");

libs/utils/src/main/java/com/akto/testing/ApiExecutor.java:179

  • [nitpick] Converting header values to uppercase may distort their original semantics. It's better to return the raw header value and rely on equalsIgnoreCase when comparing.
                    return values.get(0).toUpperCase();

if (waitLoopCount > 10) { // If no data for a while, break
break;
}
Thread.sleep(100);
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

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

[nitpick] A tight loop with Thread.sleep(100) can still be inefficient under high concurrency. Consider using non-blocking I/O with timeouts or a scheduler to avoid idle CPU cycles.

Copilot uses AI. Check for mistakes.

} else {
// Avoid tight CPU loop; sleep briefly
waitLoopCount++;
if (waitLoopCount > 10) { // If no data for a while, break
Copy link
Contributor

Choose a reason for hiding this comment

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

If there is an initial delay of 1 second before sending the response, then this will exit.

}
return sb.toString();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

also are we not removing content-type from below sse session used for mcp

    Headers headers = new Headers.Builder()
            .add("Accept", "*")
            .add("Content-Type", "text/event-stream")
            .build();

@krngrover94 krngrover94 self-requested a review June 18, 2025 07:37
@notshivansh notshivansh merged commit f6df3e7 into master Jun 18, 2025
2 of 3 checks passed
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.

2 participants