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

HTTP2 bug fix #100

Merged
merged 11 commits into from
Sep 3, 2024
Merged

HTTP2 bug fix #100

merged 11 commits into from
Sep 3, 2024

Conversation

BatMiles
Copy link
Member

What does this PR do?

  • fixes the HTTP2 crash we were seeing on some second page scans
  • fixes the session recording test
  • uses URL built-in instead of deprecated library function
  • removes some unused util functions and collector variables
  • refactors a number of files into a new subfolder
  • creates a new hasOwnProperty helper function
  • various minor improvements: function renaming, removing commented-out logs, whitespace, etc

Why are we doing this? How does it help us?

Crashes are bad

How/where should this be tested?

On it's own and in conjunction with the lambda

What are potential areas for future improvement? Are there any dependencies (especially on 3rd party code)?

There's plenty more refactoring to do, but I figured this PR was getting large enough

What are the relevant tickets, tasks, or documents?

https://calmatters.atlassian.net/jira/software/projects/PRD/boards/15?assignee=712020%3A97109b4a-04d7-464a-b1ce-adb6439221fb&selectedIssue=PRD-7402

Have you done the following, if applicable:

(optional: add explanation between parentheses)

  • Tested manually
  • Checked for performance implications? ( )
  • Checked for security vulnerabilities? ( )
  • Added/updated documentation? ( N/A)
  • Added/updated tests

@BatMiles BatMiles requested a review from dphiffer August 30, 2024 15:25
.map((link:LinkObject) => link.href)
.map(href => {
return links_with_duplicates.find(link => link.href === href);
});
Copy link
Member

Choose a reason for hiding this comment

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

This is much more readable, thank you. My first impression of this is the same: links gets set to dedupedLinkArray filtered for links that also exist in the original input array. Seems like an unnecessary check since dedupedLinkArray is already derived from links_with_duplicates.

Copy link
Member

@dphiffer dphiffer left a comment

Choose a reason for hiding this comment

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

This is great, I'm so glad that you discovered the (cookie) cache clearing trick!

@BatMiles BatMiles merged commit 1799907 into main Sep 3, 2024
1 check passed
@BatMiles BatMiles deleted the http-fix branch September 3, 2024 14:07
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