-
Notifications
You must be signed in to change notification settings - Fork 16
[TE-28689] add-on for getting status code from network logs. #196
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
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the Maven project version, introduce a new action to retrieve HTTP status codes, refactor network tracking to consolidate request and response data handling, and significantly enhance the utility class for managing network data. The focus shifts to handling headers and status codes, with improved error handling and logging throughout. Changes
Sequence Diagram(s)sequenceDiagram
participant StartTracking
participant ResponseDataUtilities
participant Network
participant Logger
StartTracking->>Network: Intercept request
Network-->>StartTracking: Provide request URL, method, headers
StartTracking->>ResponseDataUtilities: addRequestHeadersData(runId, headers, logger)
StartTracking->>Network: Wait for response
Network-->>StartTracking: Provide response status, headers, body
StartTracking->>ResponseDataUtilities: addAllNetworkData(runId, statusCode, headers, responseBody, logger)
StartTracking->>Logger: Log all network data
sequenceDiagram
participant GetStatusCode
participant ResponseDataUtilities
participant Logger
GetStatusCode->>ResponseDataUtilities: getStatusCodeData(runId, logger)
ResponseDataUtilities-->>GetStatusCode: Return status code or error
GetStatusCode->>Logger: Log result
GetStatusCode->>RuntimeVar: Store status code
sequenceDiagram
participant GetDataFromRequestBody
participant ResponseDataUtilities
participant Logger
GetDataFromRequestBody->>ResponseDataUtilities: getSpecificHeaderValue(runId, headerKey, logger)
ResponseDataUtilities-->>GetDataFromRequestBody: Return header value or error
GetDataFromRequestBody->>Logger: Log header value
GetDataFromRequestBody->>RuntimeVar: Store header value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
16-26: Add synchronization to prevent concurrent file‐write conflictsA search across the codebase shows no existing locks or
synchronizedblocks in ResponseDataUtilities or FileUtilities, so simultaneous calls to saveAllData(runId, …) can corrupt the file for the same runId.Please update store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (lines 16–26) to serialize access, for example:
• Use a per-runId lock map or a static lock around FileUtilities.writeToFile
• Or acquire a FileChannel.lock() before writingExample sketch:
private static final ConcurrentMap<Long, Object> runLocks = new ConcurrentHashMap<>(); private static void saveAllData(Long runId, JsonObject jsonObject, Logger logger) throws Exception { Object lock = runLocks.computeIfAbsent(runId, id -> new Object()); synchronized (lock) { logger.info("Saving data for the current testcase " + jsonObject); String json = gson.toJson(jsonObject); String encoded = Base64.getEncoder().encodeToString(json.getBytes()); FileUtilities.writeToFile(runId, encoded); } }– Adjust FileUtilities if it already manages files without locking
– Clean up runLocks entries if you expect many runIds
🧹 Nitpick comments (4)
store_network_logs_data/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (1)
18-22: Consider renaming the class to match its actual functionality.The class name
GetDataFromRequestBodysuggests it extracts data from request body, but the action text and implementation clearly indicate it extracts header values. This naming inconsistency could confuse users and developers.Consider renaming the class to
GetDataFromRequestHeaderorExtractRequestHeaderValueto better reflect its actual functionality.store_network_logs_data/src/main/java/com/testsigma/addons/web/StartTracking.java (1)
89-97: Fallback header mechanism may not represent actual request headers.The fallback mechanism creates synthetic headers (
:method:,:path:,:scheme:) when actual headers are missing. While this prevents empty headers, these synthetic headers may not accurately represent the actual HTTP request headers sent by the browser.Consider logging a warning when using fallback headers to make it clear that these are not the actual request headers:
+ logger.warn("Using fallback headers as actual request headers were not available");store_network_logs_data/src/main/java/com/testsigma/addons/web/GetStatusCode.java (2)
31-31: Remove unused variable declaration.The
resultvariable is declared but never used in the method. The method directly returnsResult.SUCCESSorResult.FAILED.- Result result = Result.SUCCESS;
15-17: Consider making the runtime variable name configurable.The action text hardcodes the runtime variable name to "status_code", but the implementation allows for a configurable variable name via the
statusCodeVariablefield. This could confuse users who expect to use a different variable name.Consider updating the action text to be more generic:
-@Action(actionText = "get the status code of the stored URL and store it in runtime variable status_code", +@Action(actionText = "get the status code of the stored URL and store it in runtime variable variable_name",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
store_network_logs_data/pom.xml(1 hunks)store_network_logs_data/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java(2 hunks)store_network_logs_data/src/main/java/com/testsigma/addons/web/GetStatusCode.java(1 hunks)store_network_logs_data/src/main/java/com/testsigma/addons/web/StartTracking.java(5 hunks)store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
store_network_logs_data/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (1)
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
ResponseDataUtilities(12-217)
store_network_logs_data/src/main/java/com/testsigma/addons/web/GetStatusCode.java (3)
store_network_logs_data/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (1)
Data(17-76)store_network_logs_data/src/main/java/com/testsigma/addons/web/StartTracking.java (1)
Data(23-157)store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
ResponseDataUtilities(12-217)
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/FileUtilities.java (1)
FileUtilities(10-71)
🔇 Additional comments (7)
store_network_logs_data/pom.xml (1)
9-9: Version update looks appropriate.The version increment from 1.0.2 to 1.0.4 aligns well with the significant refactoring and new functionality being introduced in this PR.
store_network_logs_data/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (1)
48-75: Excellent refactoring with improved error handling.The shift from complex JSON parsing to direct header extraction using the utility method is much cleaner and more maintainable. The error handling is comprehensive with appropriate logging.
store_network_logs_data/src/main/java/com/testsigma/addons/web/StartTracking.java (2)
56-57: Verify the necessity of the large buffer size.The buffer size is set to 100MB (100000000 bytes), which is quite large. Ensure this is necessary for your use case and won't cause memory issues.
Consider if this buffer size is appropriate for typical network responses or if it could be reduced to a more reasonable value like 10MB (10000000).
124-137: Excellent consolidation of network data storage.The unified approach to storing status code, request headers, and response body in a single JSON object is a significant improvement. This addresses the file overwriting issue mentioned in the PR objectives by consolidating all data into one operation.
store_network_logs_data/src/main/java/com/testsigma/addons/web/GetStatusCode.java (1)
28-48: Well-implemented status code extraction functionality.The implementation is clean, focused, and handles errors appropriately. The logging provides good visibility into the execution flow, and the utility method integration is seamless.
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (2)
140-171: Well-implemented header value extraction with robust parsing.The
getSpecificHeaderValuemethod provides robust parsing of header strings with good error handling and case-insensitive matching. The logging is comprehensive for debugging purposes.
192-215: Solid status code data retrieval implementation.The status code retrieval logic is well-implemented with appropriate error handling, logging, and fallback behavior (returning -1 when no data is found). This supports the PR objective of adding status code extraction functionality.
| devTool.addListener(Network.requestWillBeSent(), request -> { | ||
| String requestUrl = request.getRequest().getUrl(); | ||
|
|
||
| request.getRequest().getHeaders(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove or complete the incomplete statement.
Line 67 contains an incomplete statement request.getRequest().getHeaders(); that doesn't assign the result to any variable or perform any action. This appears to be leftover code from refactoring.
- request.getRequest().getHeaders();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| request.getRequest().getHeaders(); | |
| // (The redundant call to request.getRequest().getHeaders() has been removed) |
🤖 Prompt for AI Agents
In
store_network_logs_data/src/main/java/com/testsigma/addons/web/StartTracking.java
at line 67, the statement `request.getRequest().getHeaders();` is incomplete and
unused. Remove this line entirely or assign its result to a variable if headers
are needed later; otherwise, deleting it will clean up leftover code from
refactoring.
| public static String getResponseBody(Long runId, Logger logger) throws Exception { | ||
| String responseBody = getResponseBodyData(runId, logger).getAsString(); | ||
| clearResponseDataByRunId(runId); | ||
| return responseBody; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Inconsistent data clearing behavior across methods.
Some methods like getResponseBody, getRequestHeaders, and getStatusCode automatically clear the data after reading, while others like getStatusCodeData and getRequestHeadersData don't. This inconsistency could lead to unexpected behavior.
Consider either:
- Making all "get" methods non-destructive and providing separate "clear" methods
- Creating separate methods with clear naming like
getAndClearStatusCodevsgetStatusCode - Adding a boolean parameter to control clearing behavior
Also applies to: 134-138, 182-190
🤖 Prompt for AI Agents
In
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java
around lines 65-68, 134-138, and 182-190, the data clearing behavior after
reading is inconsistent across methods. To fix this, refactor all "get" methods
to be non-destructive by removing automatic clearing, and implement separate
clearly named methods like "getAndClearStatusCode" or add a boolean parameter to
control whether the data should be cleared after retrieval. This will ensure
consistent and predictable behavior across all data access methods.
| public static void addRequestHeadersData(Long runId, String requestHeaders, Logger logger) throws Exception { | ||
| JsonObject jsonObject = new JsonObject(); | ||
| JsonArray jsonArray = new JsonArray(); | ||
| jsonArray.add(requestBody); | ||
| jsonObject.add("requestBody", jsonArray); | ||
| jsonArray.add(requestHeaders); | ||
| jsonObject.add("requestHeaders", jsonArray); | ||
| saveAllData(runId, jsonObject, logger); | ||
| } | ||
|
|
||
| public static String getRequestBody(Long runId, com.testsigma.sdk.Logger logger) throws Exception { | ||
| String requestBody = getRequestBodyData(runId, logger).getAsString(); | ||
| public static void addStatusCodeAndRequestHeadersData(Long runId, int statusCode, String requestHeaders, Logger logger) throws Exception { | ||
| JsonObject jsonObject = new JsonObject(); | ||
| jsonObject.addProperty("statusCode", statusCode); | ||
| JsonArray jsonArray = new JsonArray(); | ||
| jsonArray.add(requestHeaders); | ||
| jsonObject.add("requestHeaders", jsonArray); | ||
| saveAllData(runId, jsonObject, logger); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Method naming and proliferation concerns.
The class has multiple similar methods with slight variations (addRequestHeadersData, addStatusCodeAndRequestHeadersData, addAllNetworkData, saveAllNetworkData) which could lead to confusion about which method to use when.
Consider consolidating these methods or providing clearer naming conventions. For example:
addRequestHeadersOnlyaddStatusCodeAndHeadersaddCompleteNetworkDatasaveNetworkData(for pre-built JsonObject)
🤖 Prompt for AI Agents
In
store_network_logs_data/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java
around lines 98 to 113, there are multiple similar methods with overlapping
functionality and unclear naming. Refactor by consolidating these methods into
fewer, more clearly named methods such as addRequestHeadersOnly,
addStatusCodeAndHeaders, addCompleteNetworkData, and saveNetworkData for
pre-built JsonObjects. This will reduce confusion and improve code clarity by
making method purposes explicit and avoiding redundant variations.
Jira
https://testsigma.atlassian.net/browse/TE-28689
https://testsigma.atlassian.net/browse/TE-28600
Addon Name: Store Network Logs Data
account : appini.akhil@testsigma.com
Jarvis Link: https://jarvis.testsigma.com/ui/tenants/2817/addons
fix
Summary by CodeRabbit
New Features
Refactor
Chores