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

chore: handle 408 error and show message [gh-824] #825

Merged
merged 5 commits into from
Aug 14, 2023

Conversation

nahuelon
Copy link
Contributor

@nahuelon nahuelon commented Aug 14, 2023

I hereby confirm that I followed the code guidelines found at engineering guidelines

Affected Components

  • CLI
  • Create CLI
  • Test
  • Docs
  • Examples
  • Other

Notes for the Reviewer

This PRs makes use of a response axios interceptor to catch 408 errors and print a user-friendly message.

image

I tried to add unit-tests for axios requests and interceptors here, but I had to __mocks__ axios resulting in issues with the e2e tests. That's why I moved the interceptors handler to separated functions to see if I could spyOn or mock them (without success).

TODO: try to test interceptors. FYI, mocking axios worked to test request but it require some E2E tests adjustments.

Resolves #824

New Dependency Submission

@nahuelon nahuelon marked this pull request as ready for review August 14, 2023 13:37
@clample
Copy link
Collaborator

clample commented Aug 14, 2023

The interceptor changes look good to me 👍

I'm not sure about the runtimes e2e test. We'll need to update the snapshot whenever we update runtimes. Could you move it out of this PR since it doesn't seem needed to handle the 408 issue?

Copy link
Member

@umutuzgur umutuzgur left a comment

Choose a reason for hiding this comment

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

LGTM

@nahuelon nahuelon merged commit 092f82c into main Aug 14, 2023
3 checks passed
@nahuelon nahuelon deleted the nahuelon/gh-824/handle-408-error branch August 14, 2023 16:11
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.

story: better error message when a user recieves 408
3 participants