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.

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

Fix 'cache is not defined' TypeErrors in cachePut/cacheGet #8788

Merged
merged 4 commits into from
Sep 24, 2019
Merged

Fix 'cache is not defined' TypeErrors in cachePut/cacheGet #8788

merged 4 commits into from
Sep 24, 2019

Conversation

vakila
Copy link

@vakila vakila commented Sep 21, 2019 •
edited
Loading

Following on from #8780, move catches to end of the promise chain to avoid TypeError: cache is not defined in the subsequent thens.

Launch checklist:

  • describe changes
  • add tests(s)

@kkaefer kkaefer added this to the release-ristretto milestone Sep 23, 2019
@vakila
Copy link
Author

vakila commented Sep 23, 2019

To really cover our bases here I think we should add some unit tests to make sure errors are being handled as expected - I'm going to take a stab at that now.

@kkaefer kkaefer mentioned this pull request Sep 24, 2019
Merged
@kkaefer kkaefer removed this from the release-ristretto milestone Sep 24, 2019
@vakila
Copy link
Author

vakila commented Sep 24, 2019

Added some basic tests for cacheGet. cachePut still needs more tests but may need to be refactored to make testing easier - will follow up on that in a separate PR

mourner
mourner approved these changes Sep 24, 2019
View reviewed changes
Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

🚢 Thanks for adding such a nice unit test!

@vakila vakila merged commit 30db469 into master Sep 24, 2019
@vakila vakila deleted the firefox-security-error-continued branch September 24, 2019 17:16
kkaefer pushed a commit that referenced this pull request Sep 25, 2019
@kkaefer
Fix 'cache is not defined' errors in tile_request_cache.js & add tests (
6f547c3 
#8788)

* Move cacheGet/Put catches to end of chain to avoid 'cache is not defined' TypeErrors
* Add some tests for tile_request_cache
@kkaefer kkaefer mentioned this pull request Sep 25, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asheemmamoowala asheemmamoowala asheemmamoowala approved these changes

@mourner mourner mourner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vakila @mourner @asheemmamoowala @kkaefer