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

Handle redirects from the server #135

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Handle redirects from the server #135

merged 1 commit into from
Feb 7, 2023

Conversation

mike-engel
Copy link
Collaborator

Closes #134

When we got rid of request in #62, we accidentally removed the ability to follow redirects, since node's http.request method doesn't have the ability to follow redirects. This will re-call the handler function when a status code is one of the four redirect types using the Location header from the response headers.

This also makes a few updates to how tests run:

  • Makes all of the request tests run serially. This fixes fragile issues with race-conditions since we're stubbing the global https node module
    • This makes the tests a little slower, but is much more resilient to changes
  • Combines the app and track api request tests into a single file
  • Updates the browserlists definitions (transitive dependency) to get rid of the annoying errors in the console

@mike-engel mike-engel added the bug label Feb 7, 2023
@mike-engel mike-engel requested a review from a team February 7, 2023 09:51
@mike-engel mike-engel self-assigned this Feb 7, 2023
@mike-engel mike-engel mentioned this pull request Feb 7, 2023
@mike-engel mike-engel merged commit 6b054f0 into main Feb 7, 2023
@mike-engel mike-engel deleted the handle-redirects branch February 7, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

301 Unknown error
2 participants