Skip to content

Add header check in ads #44

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 6, 2023
Merged

Add header check in ads #44

merged 2 commits into from
Apr 6, 2023

Conversation

@stanleegoodspeed
Copy link
Collaborator

@stanleegoodspeed stanleegoodspeed commented Apr 6, 2023

Description

Adds a check to see if the header exists prior to evaluating it's value

How to test

  1. Pull down branch
  2. Turn on error-tracking FF via services/frontend/site/featureFlags.config.json
  3. Rebuild frontend and ads service
    docker-compose build frontend ads
  4. Start the app via docker-compose up
  5. When all services are healthy, visit http://localhost
  6. Continue refreshing the homepage to generate errors
  7. In the logs for the ads service, you should see 500s
  8. You should also see 500s in the dev console of the browser

@stanleegoodspeed stanleegoodspeed added the bug Something isn't working label Apr 6, 2023
@stephen-w-pracy stephen-w-pracy self-requested a review April 6, 2023 20:15
stephen-w-pracy
stephen-w-pracy approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@stephen-w-pracy stephen-w-pracy left a comment

Choose a reason for hiding this comment

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

Top shelf.

@stanleegoodspeed stanleegoodspeed merged commit d6f3275 into main Apr 6, 2023
@stanleegoodspeed stanleegoodspeed deleted the ccole/fix-header branch April 6, 2023 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@stephen-w-pracy stephen-w-pracy stephen-w-pracy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stanleegoodspeed @stephen-w-pracy