[fix] - Cache keys implementation #8
Open
GabrielHorbach wants to merge 1 commit intoHarperFast:mainfrom
Open
Conversation
GabrielHorbach
changed the title
Contributor
jjohnson-hdb
left a comment
jjohnson-hdb
left a comment
There was a problem hiding this comment.
Use of native harper logger is preferred but looks good to me otherwise
| if (!pathname || pathname === '') { | ||
| // Fallback: use full URL path as last resort | ||
| const urlPath = new URL(request.url, `http://${request.headers.get('host') || 'localhost'}`).pathname; | ||
| console.warn('[CACHE KEY WARNING] Empty pathname, using URL path:', urlPath); |
Contributor
There was a problem hiding this comment.
Suggested change
| console.warn('[CACHE KEY WARNING] Empty pathname, using URL path:', urlPath); | |
| logger.warn('[CACHE KEY WARNING] Empty pathname, using URL path:', urlPath); |
|
|
||
| // Validate cache key is not empty - critical safety check | ||
| if (!cacheKeyString || cacheKeyString === '{}' || cacheKeyString === 'null') { | ||
| console.error('[CACHE KEY ERROR] Invalid cache key generated:', { |
Contributor
There was a problem hiding this comment.
Suggested change
| console.error('[CACHE KEY ERROR] Invalid cache key generated:', { | |
| logger.error('[CACHE KEY ERROR] Invalid cache key generated:', { |
|
|
||
| // Debug logging | ||
| if (process.env.DEBUG_CACHE_KEYS === 'true') { | ||
| console.log('[CACHE KEY]', { |
Contributor
There was a problem hiding this comment.
Suggested change
| console.log('[CACHE KEY]', { | |
| logger.info('[CACHE KEY]', { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cache Key Implementation Fix
Problem
After migrating from
@layer0/coreto@harperdb/http-router, the application experienced cache-related issues when using custom cache keys:.replace()failuresRoot Cause: The cache key generation in
@harperdb/http-routerwas fundamentally different from@layer0/core's proven implementation, causing cache key collisions where different requests generated the same cache key (serving wrong content) or same requests generated different cache keys (cache misses, serving stale content).Solution
Updated the cache key generation in
@harperdb/http-router/index.jsto match@layer0/core's approach:Query Parameter Processing: Changed from only adding whitelisted params to starting with ALL params and deleting non-whitelisted ones (matching
excludeAllQueryParametersExceptbehavior)Default Headers: Always include
hostandaccept-encodingheaders in cache key (matchinggetDefaultHeaders)Deterministic Format: Switched from string concatenation to
JSON.stringifywith sorted arrays of[key, value]pairs, ensuring consistent ordering regardless of parameter orderCookie Parsing: Improved parsing to handle values containing
=charactersPathname Validation: Added validation to ensure pathname is always present and valid