Skipped Reporting of 302 and Request abort error #4881

SukhvirKooner · 2025-01-28T17:24:36Z

Summary

Redirect requests (301/302)

Before

After

I have successfully resolved the 302 redirect request issue by updating the Axios error handling logic. Specifically, I added 302 to the list of status codes that should bypass error logging:
(error.response && [302, 403, 404, 405, 412].includes(error.response.status))
This ensures that when a 302 status is returned, Axios will handle it gracefully without triggering unnecessary error logs or forwarding it to Sentry.

Request aborted Error

Error ScreenShort

I have made the key changes to the original code to handle the Error particular during navigation:

Page State Tracking:

let isNavigating = false;
let isReloading = false;
let pendingRequests = new Set();

Added variables to track navigation state and keep track of pending requests
This helps manage network requests during page transitions

Abort Controller Integration:
const abortController = new AbortController();

Added a central AbortController to cancel pending requests

Navigation Event Handling:

window.addEventListener('navigate', () => {
  isNavigating = true;
  abortController.abort();
  setTimeout(() => {
    isNavigating = false;
  }, 100);
});

window.addEventListener('beforeunload', () => {
  abortController.abort();
});

Listens for navigation events and aborts pending requests
Uses a timeout to reset the navigation state
Handles page unload events

Request Tracking in Interceptors:

client.interceptors.request.use(config => {
  config.signal = abortController.signal;
  pendingRequests.add(config);
  return config;
});

Adds abort signal to each request
Tracks pending requests in the Set

Enhanced Error Handling:

if (
  isNavigating || 
  isReloading ||
  axios.isCancel(error) ||
  error.code === 'ERR_CANCELED' ||
  error.code === 'ECONNABORTED' ||
  (error.response && [302, 403, 404, 405, 412].includes(error.response.status))
) {
  return Promise.reject(error);
}

Added comprehensive error checking
Silently handles navigation-related cancellations and specific HTTP status codes
Added 302 status code to the suppressed errors list

Cleanup Function:

export function cleanupRequests() {
  abortController.abort();
  pendingRequests.clear();
}

Added function to clean up pending requests
Useful when components unmount

Despite these suppression mechanisms, legitimate errors unrelated to Request aborted or Redirect requests (301/302) will still be logged and captured in Sentry for further monitoring and troubleshooting.

akolson · 2025-02-03T15:16:15Z

contentcuration/contentcuration/frontend/shared/client.js

+export function cleanupRequests() {
+  abortController.abort();
+  pendingRequests.clear();
+}


Is this being used anywhere?

The exported cleanupRequests function is meant to be called when a component unmounts. It aborts any pending requests and clears the pendingRequests set to prevent memory leaks or unwanted side effects.

Right! Unfortunately I don't see any usages of this function. Makes me wonder whether we really need it, particularly within the scope of this task?

Got it! I'll go ahead and remove it.

akolson · 2025-02-03T15:35:32Z

contentcuration/contentcuration/frontend/shared/client.js

+  // Reset the flag after navigation is likely complete
+  setTimeout(() => {
+    isNavigating = false;
+  }, 100);


I don't think the use of a delay here is a reliable way to set the navigation to false. I think it would be better to do this after an abort has been confirmed. You could consider adding an event listener to an controller.signal instance and do the setting there?

You're absolutely right—relying on a fixed delay is not a reliable way to reset the navigation flag. Using an event listener on the AbortController.signal makes a lot more sense since it ensures the state update happens precisely when the abort is confirmed. I'll update as you suggested.

akolson · 2025-02-04T14:15:17Z

contentcuration/contentcuration/frontend/shared/client.js

+    if (
+      isNavigating ||
+      isReloading ||
+      axios.isCancel(error) ||


Let's import the isCancel instead so we can clear the linting error below ie;
import {isCancel} from 'axios' and this line changes to isCancel(error) ||

Thank you for the suggestion! I'll make sure to update it accordingly.

akolson · 2025-02-04T14:17:28Z

contentcuration/contentcuration/frontend/shared/client.js

+    const status = error.response?.status || 0;
+
+    // Suppress contentnode query errors
+    if (url.includes('contentnode')) {


Not sure about the motivation for this. The issue description doesn't seem to suggest this.

The primary goal here is to suppress errors specifically related to content nodes, as they are a significant source of the issues we need to address.

Since the URLs containing the keyword contentnode reliably identify these errors, this condition becomes essential for targeted error suppression.

I acknowledge that this detail should have been explicitly mentioned in the issue description.

SukhvirKooner · 2025-02-05T16:34:59Z

Hi @akolson ,
I have removed the unused functions and updated the code as suggested.

akolson

Changes seem to make sense. Thanks @SukhvirKooner. I'll invite @nucleogenesis @AlexVelezLl @ozer550 incase of any further feedback before we proceed with the approval

skipped reporting of 302 and request abort error

d5ef288

MisRob requested review from akolson, nucleogenesis, AlexVelezLl and ozer550 January 28, 2025 19:16

[pre-commit.ci lite] apply automatic fixes

1052597

akolson reviewed Feb 3, 2025

View reviewed changes

akolson reviewed Feb 4, 2025

View reviewed changes

Remove unused function

246ccb7

[pre-commit.ci lite] apply automatic fixes

906fd5d

akolson reviewed Feb 6, 2025

View reviewed changes

rtibbles assigned akolson Feb 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skipped Reporting of 302 and Request abort error #4881

Skipped Reporting of 302 and Request abort error #4881

SukhvirKooner commented Jan 28, 2025 •

edited

Loading

akolson Feb 3, 2025

SukhvirKooner Feb 3, 2025

akolson Feb 4, 2025

SukhvirKooner Feb 5, 2025

akolson Feb 3, 2025

SukhvirKooner Feb 3, 2025

akolson Feb 4, 2025

SukhvirKooner Feb 5, 2025

akolson Feb 4, 2025

SukhvirKooner Feb 5, 2025

SukhvirKooner commented Feb 5, 2025

akolson left a comment •

edited

Loading

Skipped Reporting of 302 and Request abort error #4881

Are you sure you want to change the base?

Skipped Reporting of 302 and Request abort error #4881

Conversation

SukhvirKooner commented Jan 28, 2025 • edited Loading

Summary

Redirect requests (301/302)

Request aborted Error

Despite these suppression mechanisms, legitimate errors unrelated to Request aborted or Redirect requests (301/302) will still be logged and captured in Sentry for further monitoring and troubleshooting.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SukhvirKooner commented Feb 5, 2025

akolson left a comment • edited Loading

Choose a reason for hiding this comment

SukhvirKooner commented Jan 28, 2025 •

edited

Loading

akolson left a comment •

edited

Loading