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

Error handling for OG #561

Merged
merged 7 commits into from
Feb 5, 2025
Merged

Error handling for OG #561

merged 7 commits into from
Feb 5, 2025

Conversation

joshuadkitenge
Copy link
Contributor

@joshuadkitenge joshuadkitenge commented Jan 20, 2025

Description

  • Create a ogApi axiosInstance
    • To manage 404 errors
    • set the authorisation headers
    • create handleOG_APIError to handle unknown errors
    • create the retryOG_APIErrors to hanlde retries
    • refactor all of the files in api directory to use ogApi

Reasoning

  • Implement the same method as IMS.

    • This method use the Axios interceptions. This method prevent multiple calls to re fetch the token if
      multiple queries or mutations are called
  • Retries are set to 3 attempts and it doesn't retry for 403 request as that is handle in the interceptors

  • Global error handler function set in the App.tsx handles any other errors

  • Reduces the amount code as the authorization header and api url are now set in the axios instance

  • Works well with the auto-refresh functionality currently implemented, as the automatic refresh is set to 1 minute.

Testing instructions

  • Review code
  • Check Actions build
  • Review changes to test coverage
  • {more steps here}

Agile board tracking

closes DSEGOG-359

- Create a ogApi axiosInstance
    - To manage 404 errors
    - set the authorisation headers
    - create handleOG_APIError to handle unknown errors
    - create the retryOG_APIErrors to hanlde retries
    - refactor all of the files in api directory to use ogApi
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

Attention: Patch coverage is 83.53659% with 54 lines in your changes missing coverage. Please review.

Project coverage is 97.64%. Comparing base (e2cbc62) to head (c6ea1d8).
Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
src/api/api.tsx 48.21% 29 Missing ⚠️
src/settingsMenuItems.component.tsx 66.66% 8 Missing ⚠️
src/App.tsx 16.66% 5 Missing ⚠️
src/functions/functionsDialog.component.tsx 40.00% 3 Missing ⚠️
src/session/sessionSaveButtons.component.tsx 76.92% 3 Missing ⚠️
src/api/export.tsx 89.47% 2 Missing ⚠️
...rc/filtering/favouriteFilterDialogue.component.tsx 86.66% 2 Missing ⚠️
src/session/sessionDialogue.component.tsx 84.61% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #561      +/-   ##
===========================================
- Coverage    97.86%   97.64%   -0.22%     
===========================================
  Files           94       97       +3     
  Lines        12531    12463      -68     
  Branches      2025     2043      +18     
===========================================
- Hits         12263    12170      -93     
- Misses         266      291      +25     
  Partials         2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joshuadkitenge joshuadkitenge force-pushed the DSEGOG-359-error-handling branch from 1c543c8 to cb3474d Compare January 21, 2025 11:06
- minor styling changes to make dialogs mutation dialogs similar
@joshuadkitenge joshuadkitenge force-pushed the DSEGOG-359-error-handling branch 3 times, most recently from a84961f to d33f462 Compare January 21, 2025 14:02
@joshuadkitenge joshuadkitenge force-pushed the DSEGOG-359-error-handling branch 4 times, most recently from d8234f5 to 0f1bd34 Compare January 21, 2025 16:02
- This test was failing due to the "PREFERRED_COLOUR_MAP_PREFERENCE_NAME"
    being imported from the MSW handlers. This meant that the imports
    in the settingsMenuItems had been called before the test started running.
    Vitest doesn't allow you to mock an import if it is called before the
    test file as it is already cached.

- The solution was to move the "PREFERRED_COLOUR_MAP_PREFERENCE_NAME"
    variable to the app.types.tsx
@joshuadkitenge joshuadkitenge force-pushed the DSEGOG-359-error-handling branch from 0f1bd34 to 47d7788 Compare January 21, 2025 18:00
@joshuadkitenge joshuadkitenge requested review from louise-davies and removed request for louise-davies January 22, 2025 10:01
@joshuadkitenge joshuadkitenge marked this pull request as ready for review January 23, 2025 14:24
Copy link
Member

@louise-davies louise-davies left a comment

Choose a reason for hiding this comment

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

Seems to be working well, I was able to trigger some errors using a mocking extension and it correctly caused SG to either display error toasts or prompt a token revalidation. Just a couple minor comments :)

src/handleOG_APIError.ts Outdated Show resolved Hide resolved
src/api/sessions.tsx Outdated Show resolved Hide resolved
Copy link
Member

@louise-davies louise-davies left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@joshuadkitenge joshuadkitenge merged commit e3eb32d into develop Feb 5, 2025
6 checks passed
@joshuadkitenge joshuadkitenge deleted the DSEGOG-359-error-handling branch February 5, 2025 09:51
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