Skip to content

#2632 fix webpage merge with top-level content and no views - #2648

Open
shoemoney wants to merge 3 commits into
Accenture:developfrom
shoemoney:bug/2632-webpage-2022-mergecode-typeerror
Open

shoemoney wants to merge 3 commits into
Accenture:developfrom
shoemoney:bug/2632-webpage-2022-mergecode-typeerror

Conversation

@shoemoney

@shoemoney shoemoney commented Sep 25, 2026 •

Copy link
Copy Markdown

PR details

What changes did you make? (Give an overview)

Addresses #2632. A webpage with top-level content and no views can be skipped during deployment: _mergeCode loads its HTML into metadata.content, then reads metadata.views.html.content when building the file list. This throws and is reported as a missing content block.

Use metadata.content for that entry. The views.html.content path keeps its existing behavior.

Further details (optional)

The regression uses rlilov's sanitized server response. It checks extraction, merge and the deployment dependency scan. A second test covers the existing views.html.content fixture.

This is a metadata-shape issue. The response shape differs between BUs in the reporter's tenant; creation year does not identify affected assets.

Validation: 29 asset tests passed on Node 22; both webpage regressions passed on Node 24; changed-file ESLint passed. Restoring the original file-list read makes the recorded-response test fail with the expected TypeError. The full suite had 554 passing, 21 pending and two init-tooling failures; both failures also occur with the original PR-head files. I have not run the updated tests on Windows or against a live SFMC tenant. The reporter's earlier tenant comparison used converted local files; the later comment supplies the actual server response used here.

AI assistance was used to update the tests and draft this description.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • test scripts updated
  • Wiki updated (if applicable)

@shoemoney

Copy link
Copy Markdown
Author

The coverage job on e2c5b0bf passed 313 tests with 16 pending, including the new webpage regression. The six Node 20–25 lint/test jobs also passed.

The failure is in Report coverage: the job received PullRequests: read, then the report action attempted issues.createComment and received Resource not accessible by integration. The coverage workflow is unchanged from base commit 6b316b85. The expired base-coverage artifact is a separate, nonfatal warning.

This leaves coverage-comment publication blocked by the workflow token permissions; I found no PR-code change needed for this failure.

@shoemoney

shoemoney commented Sep 25, 2026 •

Copy link
Copy Markdown
Author

The failing Test and report job is not a test failure: the coverage-comment action (sidx1024/report-nyc-coverage-github-action) dies with HttpError: Resource not accessible by integration when trying to post the coverage comment from a fork PR (classic token-permission limitation). The actual test suite passes on Node 20–24 (see the green lint & test matrix). No changes needed on the patch side.

@JoernBerkefeld

Copy link
Copy Markdown
Contributor

hi Jeremy, i dont understand this PR nor the underlying issue.
are you stating that current assets of type assetType.name==webpage are not correctly mapped?
current as in "anything created after 2022"?

i tested myself and couldnt reproduce your problem. please let me know. happy to fix whats actually broken but I cant merge this PR without more details

@JoernBerkefeld JoernBerkefeld added the question Further information is requested label Oct 4, 2026
@shoemoney

Copy link
Copy Markdown
Author

@JoernBerkefeld I overstated the scope in the description. The condition I reproduced is the metadata/file shape, not the asset's creation date: assetType.name === "webpage", no views object, and content.asset-asset-meta.html on disk. I have not verified this against a live SFMC tenant or established that all assets created after 2022 have that shape.

_mergeCode without templateName succeeds on both base and this PR. The failure is in the deployment dependency scan: _getUpsertOrderAndSkipMissing calls replaceCbReference, which passes the customer key as templateName. That reaches the file-list entry's read of metadata.views.html.content after loading the HTML into metadata.content.

This runs locally from the repository root after npm ci, without credentials or API calls:

// Save as repro.mjs, then run node repro.mjs
import './lib/index.js';
import Asset from './lib/metadataTypes/Asset.js';
import { mkdtemp, mkdir, writeFile, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
const dir = await mkdtemp(join(tmpdir(), 'sfmc-webpage-'));
try {
    await mkdir(join(dir, 'asset/asset/repro'), { recursive: true });
    await writeFile(join(dir, 'asset/asset/repro/content.asset-asset-meta.html'),
        '<p>No content-block references</p>');
    const item = { customerKey: 'repro', assetType: { name: 'webpage' } };
    console.log(Object.keys(await Asset._getUpsertOrderAndSkipMissing({ repro: item }, dir)));
} finally {
    await rm(dir, { recursive: true, force: true });
}

On current base 6b316b85, this logs content block Cannot read properties of undefined (reading 'html') ... was not found and prints []. On PR head e2c5b0bf, it prints [ 'repro' ]. I also reran the existing regression with only Asset.js reverted to base: one failure on base, one pass with the fix. The description now distinguishes this local reproduction from the reporter's tenant-level report.

@JoernBerkefeld
JoernBerkefeld changed the base branch from main to develop October 5, 2026 07:57
@JoernBerkefeld

Copy link
Copy Markdown
Contributor

I changed your base branch to develop. That, among other things changes where webpage type is stored (prep for v10).

What did you mean by "I have not verified this against a live SFMC tenant or established that all assets created after 2022 have that shape."

Also, could you provide a sample mcdev command that you ran which would cause this issue? Retrieve and deploy worked in my test. Did you see it with build*?

@JoernBerkefeld

JoernBerkefeld commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

To be clear, your code sample above is not what I'm looking for. I need a real end-2-end

@shoemoney
shoemoney force-pushed the bug/2632-webpage-2022-mergecode-typeerror branch from e2c5b0b to ee4e0cd Compare October 5, 2026 14:10
@shoemoney

Copy link
Copy Markdown
Author

@JoernBerkefeld thanks for retargeting. I've rebased onto develop, so the conflicts are gone and the diff is back to Asset.js (one line) plus the test.

What "not verified against a live tenant" meant: I don't have an SFMC tenant. I reproduced it at the unit level from the asset shape @rlilov described in #2632, not from my own end-to-end run. I can't give you a live log myself.

Command: the steps are the ones @rlilov reported on 9.0.3 in #2632:

mcdev retrieve <cred>/<BU> -m "asset:n:<2022+ CloudPage webpage>"
# copy retrieve/<cred>/<BU>/asset/asset/<key>/ to deploy/<cred>/<BU>/asset/asset/<key>/
mcdev deploy <cred>/<BU> asset "<key>"

Why retrieve + deploy can pass in a test: the throw needs two things together:

  1. content.asset-asset-meta.html exists in the asset folder. _extractCode on develop still writes it whenever metadata.content is set.
  2. The meta JSON has no views object.

On deploy, the pre-deploy reference check calls replaceCbReference(), which always calls _mergeCode(item, dir, subType, <key>). Because templateName is set, the 2022+ branch builds its fileList from metadata.views.html.content. If the webpage still carries a views.html object (slots, for example), it doesn't throw. If views is absent, it does. The error gets caught and reported as "content block … referenced via ContentBlockByX was not found", and the asset is skipped.

I checked this on today's develop (5d57b39): with the new test and without the fix, it fails with TypeError: Cannot read properties of undefined (reading 'html') at Asset._mergeCode (Asset.js:1723). With the fix, all 54 asset tests pass (Node 22).

@rlilov, if you can run your 2022+ webpage through develop, that would give the end-to-end confirmation I can't produce.

@rlilov

rlilov commented Oct 6, 2026

Copy link
Copy Markdown

@shoemoney , Tested end-to-end against a live SFMC tenant (child BU, sandbox) on Windows 11 / Node 24.18.0. The fix works.

Setup

  • develop @ 5d57b39 (without the fix) vs. PR #2632 fix webpage merge with top-level content and no views #2648 @ ee4e0cd (develop + fix), both run from a local clone via node lib/cli.js.
  • Test asset: an existing CloudPage webpage asset (d086c8a9-…), retrieved with develop (now under asset/webstudio/). I then converted the local files to the 2022+ layout: removed views from the JSON and put the HTML in content.asset-webstudio-meta.html. The content was changed v1 → v2.
  • Same command for both runs: mcdev deploy <cred>/<BU> asset d086c8a9-… --fromRetrieve

Run 1: develop without the fix

error:  ☇ skipping asset d086c8a9-…: content block Cannot read properties of undefined (reading 'html') that is referenced via ContentBlockByX was not found on BU nor in deployment package
info: asset upsert: 0 of 0 created / 0 of 0 updated

Run 2: PR #2648

info:  - updated asset: d086c8a9-… / MCDEV_2632_Test_Page
info: asset upsert: 0 of 0 created / 1 of 1 updated

A retrieve after the deploy returns the v2 HTML in content. So SFMC accepted the update.

@rlilov

rlilov commented Oct 6, 2026

Copy link
Copy Markdown

Other things I saw (not blockers for this PR)

  1. Same bug on retrieve. Before the fix, retrieve also logs cleanup failed: Cannot read properties of undefined (reading 'html'); manual deletion of stale extracted code files may be necessary when a 2022+ webpage already exists locally. It is the same _mergeCode line, called from the stale-file cleanup in getJsonFromFS. The PR fixes this too.
  2. The new unit test fails on Windows (the other 53 asset tests pass): ENOENT, no such file or directory 'C%3A\Users\…\Temp\mcdev-asset-webpage-2022-…\asset\webpage\testNew_asset_webpage2022\content.asset-webpage-meta.html'. The test builds an absolute path from os.tmpdir(), and the drive colon becomes %3A, so it probably passes on Linux CI only. Also, develop maps webpage to the webstudio subtype now, while the test uses asset/webpage/ and .asset-webpage-meta.
  3. Unrelated develop regression, saved sessions: the second develop run on the same machine fails before any API call with "market_audience_create", …, "http_headers_delete" is/are invalid scope(s). The first run saves the session in %APPDATA%\mcdev-nodejs\Config\sessions.json with the scopes that the server returned. The next run reloads it, and sfmc-sdk 4.0.1 rejects scopes it does not know. Removing sessions.json lets one more run work. I can open a separate issue if that helps.

@shoemoney

Copy link
Copy Markdown
Author

@rlilov thanks for the live-tenant deployment comparison and the retrieve-after-deploy confirmation. That provides the tenant evidence I could not run myself; your fixture conversion also makes the tested metadata shape explicit.

I addressed the test feedback in 9d33385: the temporary directory is now relative, avoiding the drive colon that the production path filter encodes on Windows, and the fixture uses the current webstudio folder and .asset-webstudio-meta extension. The production fix remains unchanged.

Validation here: 28 asset tests passed with Node 22.22.3, and ESLint passed for the changed test file. I have not run the revised test on Windows; that platform confirmation is still outstanding.

The saved-session scope rejection is separate from this patch. A separate issue with the sanitized error and SDK version would help keep that investigation focused.

@JoernBerkefeld

Copy link
Copy Markdown
Contributor

@rlilov - so you are saying the cloudpages created after 2022 have a different layout? ive checked this on multiple instances of SFMC and was not able to reproduce it which puzzles me quite a bit.

@rlilov
could you please provide a minimalist new json returned by a ˋ mcdev retrieve cred/bu -m asset:yourKey --api logˋ ? you should find the actual server response in the latest file created under ./logs/ - thats what i need for an actual test fixture. the current test in this PR is constructing the situation without an actual json which is not sufficient here; and not how tests are usually written in this project.

please understand that this project is used by hundreds of people and some SFMC instances produce other output then others for some reason. therefore i need to be careful to not break things for other users with this patch.

thank you both for your contribution this far. lets push a bit more to close this.

@rlilov

rlilov commented Oct 12, 2026

Copy link
Copy Markdown

@JoernBerkefeld thanks, here is the real server response.

Command (mcdev 9.0.3, child BU):

mcdev retrieve / -m asset: --api log
Raw GET asset/v1/content/assets/{id} response from ./logs/. I changed only IDs, names, emails and the key, and I shortened content. The structure is unchanged:

{
"id": 12345,
"customerKey": "testExisting_asset_webpage2022",
"objectID": "00000000-0000-0000-0000-000000000000",
"contentType": "text/html",
"assetType": { "id": 205, "name": "webpage", "displayName": "Web Page" },
"name": "testExisting_asset_webpage2022",
"owner": { "id": 1, "name": "Jane Doe", "email": "jane.doe@example.com", "userId": "1" },
"createdDate": "2026-07-02T03:27:52.677-06:00",
"createdBy": { "id": 1, "name": "Jane Doe", "email": "jane.doe@example.com", "userId": "1" },
"modifiedDate": "2026-10-07T07:02:12.693-06:00",
"modifiedBy": { "id": 2, "name": "VSCode_DevTools app user", "userId": "2" },
"enterpriseId": 1111111,
"memberId": 9999999,
"status": { "id": 1, "name": "Draft" },
"thumbnail": { "thumbnailUrl": "/v1/assets/12345/thumbnail" },
"category": { "id": 8888 },
"content": "\n\n\n%%[ SET @Greeting = "Hello" ]%%\n

%%=v(@Greeting)=%% World

\n\n\n",
"availableViews": [],
"modelVersion": 2
}
The query response for the same asset has the same shape: content at the top level, availableViews: [], and no views, meta or data.

About "2022+": I took the label from the // +++ new webpages / 2022+ +++ comment in Asset._mergeCode. The creation date is not the trigger. In the same tenant:
Business unit A: all 12 webpages have this shape. 2 of them were created in 2023, and the other 10 are UI copies of those 2 (2026). A copy keeps the shape.
Business units B and C: all webpages (2024–2026) have the normal views.html.content shape.
I do not know how the 2 original pages were created. Both have the same createdDate to the millisecond, so a bulk or API import is possible.

Limitation: I have access to one SFMC tenant only, so I cannot say how common this shape is. The fix changes only the content.html branch (the 2022+ block). That block runs only when the asset has a top-level content. The views.html.content branch is unchanged, so assets with views behave as before.

@shoemoney shoemoney changed the title #2632 fix Asset._mergeCode reading metadata.views.html.content for 2022+ webpages #2632 fix webpage merge with top-level content and no views Oct 15, 2026
@shoemoney

Copy link
Copy Markdown
Author

@JoernBerkefeld I replaced the constructed fixture with @rlilov’s recorded response and added regressions for extraction, merge, dependency scanning, and the existing views layout. The asset tests and lint pass; the two full-suite init-tooling failures also occur on the original PR head. I removed the creation-year labels because the response shape varies by BU. I haven’t tested this update on Windows or a live tenant.

@JoernBerkefeld

Copy link
Copy Markdown
Contributor

Thank you.
This pr needs to be rebased to resolve the pre-existing issues

…nt for 2022+ webpages

_mergeCode read metadata.views.html.content when building the fileList
entry for new-style (2022+) webpage assets, but those assets have no
views object at all - content lives at metadata.content, exactly as
the sibling 'template' case already does a few lines up.

This throws a TypeError (Cannot read properties of undefined, reading
'html') whenever templateName is set, which happens during a normal
deploy via Asset.replaceCbReference(). The asset gets silently skipped
with a misleading 'content block ... was not found' error instead.
@shoemoney
shoemoney force-pushed the bug/2632-webpage-2022-mergecode-typeerror branch from eeae2ca to 0a3d79e Compare October 15, 2026 10:14
@shoemoney

Copy link
Copy Markdown
Author

Rebased onto current develop (now at the 10.1.0 tip) — conflicts resolved, diff is unchanged (Asset.js, the recorded fixture, and the asset tests). Pushed as 0a3d79e. The 57 local failures I see are all INIT TOOLING npm-stub tests failing on macOS; the asset tests all pass. Letting CI grade it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

3 participants