Skip to content

[MMM] Fix 500 Internal Server Error when toggling manifest drawer#5801

Open
markpadbe wants to merge 1 commit intostagefrom
mmm-fix500error
Open

[MMM] Fix 500 Internal Server Error when toggling manifest drawer#5801
markpadbe wants to merge 1 commit intostagefrom
mmm-fix500error

Conversation

@markpadbe
Copy link
Copy Markdown
Contributor

@markpadbe markpadbe commented Apr 10, 2026

Summary

Toggling any page/manifest drawer on mmm page (i.e., clicking ".mmm-trigger" button) triggers a 500 Internal Server Error from the lambda API, followed by an uncaught TypeError: Cannot destructure property 'page' of 'mepConfig' as it is null. This update prevents normalizePath from duplicating query params on external URLs while preserving the existing entitlements param-retention behavior for known origins.

  • personalization.js: Introduce isKnownOrigin flag to distinguish AEM/HLX/adobe/localhost URLs from external ones. Known origins continue using pathname + search + hash (preserving the 97b9337 entitlements fix). External URLs reconstruct via new URL(path) to avoid duplicating query params.
  • mmm.js: Add null guard on pageData in toggleDrawer to prevent the destructure-of-null crash when fetchData returns null.
  • normalizePath.test.js: Two new tests covering external URLs with query params and #_dnt hash stripping.

Root Cause
#5450 added ${search} to the return in normalizePath to fix entitlement sheet parameters not being retained. This works correctly for AEM/HLX/adobe/localhost URLs where path is reassigned to just pathname, but for external URLs (like the lambda API), path is never reassigned and still contains the full URL with query params. Appending ${search} duplicates them. The server receives a malformed URL and returns 500. fetchData returns null, which propagates to getMepPopup(null, true) where destructuring null throws.

Steps to QA Using the Before and After Test URLS below:

  1. Scroll down to the first page with manifests found
  2. Toggle the + to the right of the page name
    Expected Results: Manifest content is retrieved and displayed
    Actual Results: spinner loads and 500 error thrown in console.

Resolves: MWPW-192357

Test URLs:

Fix normalizePath duplicating query params for external URLs.
Commit 97b9337 added ${search} to the return, which is correct
when path is stripped to pathname (AEM/HLX/adobe origins), but
duplicated params for external URLs where path retains the full URL.

Split return logic on isKnownOrigin: known origins reconstruct
from pathname+search+hash; external URLs pass through via new URL().
Add null guard in mmm.js toggleDrawer to prevent destructure-of-null
crash when fetchData returns null. Add unit tests for external URL
normalization.

Made-with: Cursor
@markpadbe markpadbe requested review from a team and vgoodric as code owners April 10, 2026 19:45
@aem-code-sync
Copy link
Copy Markdown
Contributor

aem-code-sync bot commented Apr 10, 2026

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-sync branch
Commits

@markpadbe markpadbe requested a review from denlight April 10, 2026 20:56
path = isFederal ? getFederatedUrl(path) : path;
return `${path}${search}${hash.replace(mepHash, '')}`;
if (isKnownOrigin) {
return `${path}${search}${hash.replace(mepHash, '')}`;
Copy link
Copy Markdown
Contributor

@denlight denlight Apr 14, 2026

Choose a reason for hiding this comment

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

Not sure we are addressing the case where path has search already. if it does, we are going to append search again, which is what is breaking mmm currently. I believe what was meant to be used is pathname?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question. This line (130) only runs when isKnownOrigin is true, which means path has already been reassigned to just pathname on line 121. So appending ${search} here is re-attaching the query params to the stripped pathname, not duplicating them.

The case you're describing (where path retains the full URL) is handled just below on lines 132-134, in the !isKnownOrigin branch. There we reconstruct via new URL(path) which preserves existing query params without appending anything.

@markpadbe markpadbe requested review from a team, denlight and jpratt2 April 14, 2026 17:09
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.

3 participants