web/src/plugins/logs/Index.vue (4)

126-135: LGTM! Consider adding an aria-label for accessibility.

The addition of the error details toggle button is a good improvement for user experience. It allows users to view more detailed error information when needed.

Consider adding an aria-label attribute to the button for better accessibility:

 <q-btn
   v-if="
     searchObj.data.errorMsg != '' ||
     searchObj?.data?.functionError != ''
   "
   @click="toggleErrorDetails"
   size="sm"
   data-test="logs-page-result-error-details-btn"
+  aria-label="Toggle error details"
 >{{ t("search.histogramErrorBtnLabel") }}</q-btn>

---

Line range hint 140-203: LGTM! Consider refactoring repeated code.

The changes in this section are consistent with the previous segment, which is good for maintaining code consistency. The use of optional chaining (?.) is a good practice to prevent potential errors.

Consider refactoring the repeated button code into a separate component or computed property to reduce duplication. For example:

<!-- In the template -->
<error-details-button 
  v-if="showErrorDetailsButton"
  @click="toggleErrorDetails"
/>

<!-- In the script -->
import ErrorDetailsButton from './ErrorDetailsButton.vue';

export default {
  components: {
    ErrorDetailsButton,
  },
  computed: {
    showErrorDetailsButton() {
      return this.searchObj.data.errorMsg !== '' || this.searchObj?.data?.functionError !== '';
    },
  },
  // ...
};

This would make the code more maintainable and reduce the risk of inconsistencies between different instances of the button.

---

254-257: LGTM! Consider adding error handling for missing function errors.

The addition of the SanitizedHtmlRenderer for function errors is a good improvement. It allows for secure rendering of potentially unsafe content and improves the visibility of function-specific errors.

Consider adding a fallback message or error handling in case the functionError is undefined or empty:

<SanitizedHtmlRenderer
  data-test="logs-search-detail-function-error-message"
  :htmlContent="searchObj?.data?.functionError || 'No function error details available'"
/>

This ensures that users always see some information, even if there are no specific function error details.

---

Line range hint 1-1: Consider breaking down this component for better maintainability.

The changes made in this PR are good improvements to error handling and user experience. However, this file (Index.vue) is quite large and complex, which could lead to maintainability issues in the future.

Consider breaking this component down into smaller, more focused components. This could include:

1. A separate ErrorDisplay component that handles all error-related rendering and logic.
2. A QuickModeHandler component that encapsulates the quick mode functionality.
3. Splitting the template into smaller, reusable components.

This refactoring would make the code more modular, easier to understand, and simpler to maintain. It would also make it easier to test individual pieces of functionality.

Example structure:

components/
  ErrorDisplay.vue
  QuickModeHandler.vue
  SearchBar.vue
  SearchResults.vue
Index.vue

This structure would allow Index.vue to focus on composition and high-level logic, while delegating specific functionalities to more focused components.

Configuration used: CodeRabbit UI
Review profile: CHILL

web/src/plugins/logs/Index.vue (1)

Missing handleQuickModeChange Method Implementation

The handleQuickModeChange() method mentioned in the review comment was not found in web/src/plugins/logs/Index.vue.

• toggleErrorDetails() method is present and correctly implemented.

Please ensure that the handleQuickModeChange() method is added and properly integrated into the component to support the quick mode functionality.

🔗 Analysis chain

Line range hint 1-1: Verify implementation of new and updated methods

The AI-generated summary mentions two important changes that are not directly visible in the provided code segments:

1. Addition of a new handleQuickModeChange() method
2. Update to the toggleErrorDetails() method signature

While these changes align with the observed improvements in error handling and quick mode functionality, it's important to verify their correct implementation and usage throughout the component.

To confirm these changes, please run the following script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the implementation of handleQuickModeChange and toggleErrorDetails methods

# Check for handleQuickModeChange method
echo "Checking for handleQuickModeChange method:"
ast-grep --lang vue --pattern 'handleQuickModeChange() {
  $$$
}'

# Check for toggleErrorDetails method
echo "Checking for toggleErrorDetails method:"
ast-grep --lang vue --pattern 'toggleErrorDetails() {
  $$$
}'

Length of output: 530

---

Script:

#!/bin/bash
# Verify the implementation of handleQuickModeChange and toggleErrorDetails methods using ripgrep

# Check for handleQuickModeChange method
echo "Checking for handleQuickModeChange method in web/src/plugins/logs/Index.vue:"
rg 'handleQuickModeChange\s*\(' web/src/plugins/logs/Index.vue

# Check for toggleErrorDetails method
echo "Checking for toggleErrorDetails method in web/src/plugins/logs/Index.vue:"
rg 'toggleErrorDetails\s*\(' web/src/plugins/logs/Index.vue

Length of output: 473

Configuration used: CodeRabbit UI
Review profile: CHILL

web/src/composables/useLogs.ts (1)

Line range hint 1517-3440: Refactor large and complex function for improved maintainability.

The getQueryData function is overly large (200+ lines) and complex, handling multiple responsibilities. This makes it difficult to understand, test, and maintain. Consider refactoring it into smaller, more focused functions. For example:

1. Separate data fetching logic from data processing.
2. Extract the histogram generation logic into its own function.
3. Create a separate function for handling SQL vs non-SQL modes.

Here's a high-level example of how you might refactor this:

async function getQueryData(isPagination = false) {
  try {
    await prepareQueryData(isPagination);
    const queryReq = buildQueryRequest();
    const data = await fetchData(queryReq);
    await processQueryResults(data);
    if (shouldGenerateHistogram()) {
      await generateHistogram();
    }
    updateUIState();
  } catch (error) {
    handleError(error);
  }
}

This refactoring will improve readability, testability, and maintainability of the code.

Improve error handling with specific error messages.

The current error handling uses generic error messages in some cases. Enhance the error handling to provide more specific information about what went wrong. This will make debugging easier and improve the user experience.

For example, replace:

notificationMsg.value = "Error occurred during the search operation.";

With more specific error messages based on the type of error encountered:

notificationMsg.value = `Error during search operation: ${getSpecificErrorMessage(error)}`;

function getSpecificErrorMessage(error: Error): string {
  // Logic to determine specific error message based on error type
}

Reduce direct state mutations and improve state management.

The function directly mutates the searchObj state in multiple places. This tight coupling to the global state makes the function less reusable and harder to test in isolation. Consider using a more structured state management approach, such as a reducer pattern or a state management library.

Instead of direct mutations like:

searchObj.data.functionError = "";

Consider using a dedicated state update function:

function updateSearchState(updates: Partial<typeof searchObj>) {
  // Logic to update state in a controlled manner
}

updateSearchState({ data: { functionError: "" } });

Remove or document commented-out code.

There are several blocks of commented-out code in the function. This can make the function harder to read and maintain. Either remove the commented code if it's no longer needed, or if it's kept for future reference, add a clear comment explaining why it's there and under what conditions it might be uncommented.

For example, remove or document these commented lines:

// searchObj.data.histogram.chartParams.title = "";
// if (hasAggregationFlag) {
//   searchObj.data.queryResults.total = res.data.total;
// }

Separate data fetching and processing concerns.

The function currently handles both data fetching and processing, which violates the Single Responsibility Principle. Consider separating these concerns into different functions or modules. This will make the code more modular and easier to maintain.

Create separate functions for data fetching and processing:

async function fetchQueryData(queryReq: any) {
  // Logic to fetch data
}

function processQueryData(data: any) {
  // Logic to process fetched data
}

// In getQueryData:
const data = await fetchQueryData(queryReq);
processQueryData(data);

Optimize performance for asynchronous operations.

The function performs multiple asynchronous operations sequentially, which could be optimized. Consider using Promise.all() for operations that can be performed concurrently to improve performance.

For example:

const [histogramData, pageCount] = await Promise.all([
  generateHistogramData(),
  getPageCount(queryReq)
]);

This will run both operations concurrently, potentially reducing the overall execution time.

Configuration used: CodeRabbit UI
Review profile: CHILL

web/src/composables/useLogs.ts (7)

Line range hint 1-37: Imports and initial setup look good, but consider organizing imports.

The imports are comprehensive and cover various aspects of the application. However, there's an opportunity to improve organization:

1. Group related imports together (e.g., Vue-related, utility functions, services).
2. Consider using barrel files for utility functions to reduce the number of import lines.

Example reorganization:

// Vue and related imports
import { reactive, ref, type Ref, toRaw, nextTick, onBeforeMount } from "vue";
import { useI18n } from "vue-i18n";
import { useStore } from "vuex";
import { useRouter } from "vue-router";

// Quasar imports
import { date, useQuasar } from "quasar";

// Utility imports
import { cloneDeep } from "lodash-es";
import {
  useLocalLogFilterField,
  b64EncodeUnicode,
  b64DecodeUnicode,
  // ... other utility imports
} from "@/utils/zincutils";

// Service imports
import searchService from "@/services/search";
import savedviewsService from "@/services/saved_views";

// Configuration imports
import config from "@/aws-exports";

// Type imports
import type { LogsQueryPayload } from "@/ts/interfaces/query";

---

Line range hint 39-324: State management is comprehensive but could benefit from modularization.

The defaultObject and searchObj are quite large and contain a mix of UI state, query parameters, and data fetching state. This could lead to maintenance issues as the application grows.

Consider breaking down the state into smaller, more focused objects:

1. Split UI state, query parameters, and data fetching state into separate reactive objects.
2. Use composables for specific features (e.g., useHistogram, useStreamManagement).
3. Implement a state machine for managing complex state transitions (e.g., loading, error, success states).

Example refactoring:

const uiState = reactive({
  loading: false,
  loadingHistogram: false,
  showFields: true,
  showQuery: true,
  // ... other UI-related state
});

const queryParams = reactive({
  stream: [],
  datetime: {
    startTime: 0,
    endTime: 0,
    type: 'relative',
    relativeTimePeriod: '15m'
  },
  // ... other query-related parameters
});

const dataState = reactive({
  queryResults: {},
  histogram: {},
  // ... other data-related state
});

This approach will make the code more maintainable and easier to reason about.

---

Line range hint 326-423: Helper functions are well-implemented but could be more modular.

The helper functions like useLogs, resetSearchObj, and updatedLocalLogFilterField are well-implemented. However, they are all contained within the main useLogs composable, which could lead to a very large and hard-to-maintain file.

Consider extracting some of these helper functions into separate utility files or composables:

1. Move pure functions (those that don't depend on the reactive state) to a separate utility file.
2. Create separate composables for related functionality (e.g., useSearchReset, useLocalStorage).

Example:

// searchUtils.ts
export const resetSearchObj = (searchObj) => {
  // ... reset logic
};

// useLocalStorage.ts
export const useLocalStorage = () => {
  const updatedLocalLogFilterField = () => {
    // ... local storage logic
  };

  return { updatedLocalLogFilterField };
};

// useLogs.ts
import { resetSearchObj } from './searchUtils';
import { useLocalStorage } from './useLocalStorage';

const useLogs = () => {
  const { updatedLocalLogFilterField } = useLocalStorage();
  // ... rest of the useLogs implementation
};

This approach will improve code organization and reusability.

---

Line range hint 425-1515: Core functionality is robust but complex, consider breaking it down further.

The core functionality, including functions like buildSearch, getQueryPartitions, and getPaginatedData, is comprehensive and handles various scenarios. However, the complexity of these functions makes them hard to maintain and test.

Consider the following improvements:

1. Break down large functions into smaller, more focused functions.
2. Use the Strategy pattern for different query types (SQL mode, non-SQL mode).
3. Implement error handling and logging consistently across all functions.
4. Use dependency injection for services to improve testability.

Example refactoring for buildSearch:

const buildSearchStrategy = {
  sql: (query, config) => {
    // ... SQL-specific logic
  },
  nonSql: (query, config) => {
    // ... non-SQL logic
  }
};

const buildSearch = () => {
  const strategy = searchObj.meta.sqlMode ? 'sql' : 'nonSql';
  return buildSearchStrategy[strategy](searchObj.data.query, config);
};

This approach will make the code more modular and easier to test and maintain.

Consider implementing a caching mechanism for frequently accessed data, such as stream lists and field definitions. This could significantly improve performance, especially for large datasets.

---

Line range hint 1517-2275: Function error handling improvement needed.

The changes in the getPaginatedData function introduce new error handling for function errors. However, there are some inconsistencies and potential improvements to be made.

1. The error handling for function_error is duplicated. The first check (lines 2118-2123) is redundant as it's covered by the more comprehensive check that follows.
2. The commented-out error handling code (lines 2269-2275) should be removed if it's no longer needed.

Refactor the error handling to be more concise and avoid duplication:

if (
  res.data.hasOwnProperty("function_error") &&
  res.data.function_error != ""
) {
  searchObj.data.functionError = res.data.function_error;

  if (
    res.data.hasOwnProperty("new_start_time") &&
    res.data.hasOwnProperty("new_end_time")
  ) {
    searchObj.data.functionError = getFunctionErrorMessage(
      res.data.function_error,
      res.data.new_start_time,
      res.data.new_end_time,
      store.state.timezone
    );
    // Update datetime and query parameters
    // ... (rest of the code)
  }
}

This refactoring eliminates the duplication and removes the need for the commented-out code.

---

Line range hint 3440-3448: Redundant error reset in searchAroundData function.

The searchAroundData function resets the functionError unnecessarily, as it's already being reset in the getQueryData function.

Remove the redundant reset of functionError:

const searchAroundData = (obj: any) => {
  try {
    searchObj.loading = true;
    searchObj.data.errorCode = 0;
    // Remove this line:
    // searchObj.data.functionError = "";
    const sqlContext: any = [];
    // ... rest of the function
  } catch (e: any) {
    // ... error handling
  }
};

This change will maintain consistency with the error handling approach used in other parts of the code.

---

Line range hint 1-4190: Overall assessment: Functional but complex, with room for improvement.

The useLogs composable is a comprehensive implementation that handles various aspects of log management, including querying, pagination, and state management. While it's functional, there are several areas where improvements could be made:

1. Code Organization: The file is quite large and could benefit from being split into smaller, more focused composables or utility functions.
2. State Management: Consider using a more structured approach to state management, possibly leveraging Vuex or Pinia for global state.
3. Error Handling: Implement a more consistent and robust error handling strategy throughout the composable.
4. Testability: Improve the testability of the code by using dependency injection and breaking down complex functions.
5. Performance: Consider implementing caching mechanisms for frequently accessed data.

Next steps for improvement:

1. Refactor the composable into smaller, more focused composables (e.g., useQueryBuilder, useHistogram, useStreamManagement).
2. Implement a state machine for managing complex state transitions.
3. Create a centralized error handling service to manage and log errors consistently.
4. Use TypeScript more effectively by defining and using interfaces for complex objects.
5. Implement unit tests for core functionality to ensure reliability during refactoring.

These improvements will lead to a more maintainable, testable, and performant codebase.

Configuration used: CodeRabbit UI
Review profile: CHILL

web/src/composables/useLogs.ts (2)

2269-2275: Remove commented-out code blocks.

There are two blocks of commented-out code that appear to be duplicates of the function error handling logic. These should be removed to improve code clarity and maintainability.

Consider removing these commented-out blocks:

-          // searchObj.data.functionError = "";
-          // if (
-          //   res.data.hasOwnProperty("function_error") &&
-          //   res.data.function_error
-          // ) {
-          //   searchObj.data.functionError = res.data.function_error;
-          // }

Also applies to: 2282-2288

---

Line range hint 1517-2288: Refactor error handling for better maintainability.

The error handling for function errors is currently spread across different parts of the getQueryData function. Consider refactoring this logic into a separate helper function to improve code organization and maintainability.

Create a helper function to handle function errors:

function handleFunctionError(responseData) {
  if (responseData.hasOwnProperty("function_error") && responseData.function_error !== "") {
    return responseData.function_error;
  }
  return "";
}

Then, use this helper function in the getQueryData function:

searchObj.data.functionError = "";
// ... existing code ...
if (res.data) {
  searchObj.data.functionError = handleFunctionError(res.data);
  // ... rest of the response handling ...
}

This refactoring will centralize the error handling logic and make it easier to maintain and update in the future.

Configuration used: CodeRabbit UI
Review profile: CHILL