Skip to content
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

Refactor GetBookmarkStatusFunction response logic #88

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

babakamyljanovssw
Copy link
Member

@babakamyljanovssw babakamyljanovssw commented Nov 12, 2024

We are currently observing numerous 404 NotFound responses from the GetBookmarkStatusFunction in the client Application Insights. The current logic generates two types of responses:

  • When bookmark is found:
if (bookmarks.Any())
{
    return req.CreateJsonResponse(new
    {
        error = false,
        message = "",
        bookmarkStatus = true,
    });
}
  • When bookmark is not found:
return req.CreateJsonResponse(new
{
    error = false,
    message = "",
    bookmarkStatus = false,
}, HttpStatusCode.NotFound);

In both cases, the error property is set to false, which indicates a successful operation. The only distinction is the bookmarkStatus property and the different status codes (200 vs. 404). However, the front-end app relies on the bookmarkStatus property alone to display different icons, while Application Insights on the front end logs any 400-500 status codes as failures automatically.

Suggested Improvement

To streamline this logic, I’ve updated the response to a single format which makes it generic, returning a 200 OK status code in all cases. This change removes the 404 response for the "not found" case, as it conflicts with the error = false indicator and avoids marking these responses as failures.

var bookmarkStatus = bookmarks.Any();
if (bookmarkStatus)
{
    _logger.LogInformation($"Could not find results for rule id: {ruleGuid}, and user: {userId}");
}

return req.CreateJsonResponse(new
{
    error = false,
    message = "",
    bookmarkStatus,
});

Now, the response is a unified JSON object that differs only by the bookmarkStatus property, with a consistent 200 OK status code. This will ensure Application Insights does not log these responses as failures, aligning the backend response with frontend expectations.

Logging: The conditional _logger.LogInformation also logs when no bookmarks are found in back-end, making it easier to track missing bookmarks if needed.

Application Insights
image
Figure: The same function for different rules and users are logged as failures, in case it shouldn't be failure

@babakamyljanovssw babakamyljanovssw merged commit 12b0083 into main Nov 13, 2024
3 checks passed
@babakamyljanovssw babakamyljanovssw deleted the improve-bookmark-status-function branch November 13, 2024 01:52
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