Skip to content

CMG-1093 | Contentful migration got stuck some time #1122

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

Open
chetan-contentstack wants to merge 2 commits into
base: dev
Choose a base branch
from
+3 −5
Open

CMG-1093 | Contentful migration got stuck some time #1122

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0757fd0
fix: improve file writing and ensure asset directory creation
Jul 27, 2026
86c00d7
fix(contentful): rethrow from createAssets so write failures surface
Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions api/src/services/contentful.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,7 @@ async function readFile(filePath: string, fileName: string) {
* @throws {Error} - If there is an error writing the file.
*/
async function writeOneFile(indexPath: string, fileMeta: any) {
fs.writeFile(indexPath, JSON.stringify(fileMeta), (err) => {
if (err) {
console.error("Error writing file: 3", err);
}
});
await fs.promises.writeFile(indexPath, JSON.stringify(fileMeta));
Comment thread
chetan-contentstack marked this conversation as resolved.
}

/**
Expand Down Expand Up @@ -798,6 +794,7 @@ const createAssets = async (packagePath: any, destination_stack_id: string, proj
);

await Promise.all(tasks);
await fs.promises.mkdir(assetsSave, { recursive: true });

@umesh-more-cstk umesh-more-cstk Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: (verified good) Correct and necessary. When every asset fails, saveAsset only mkdirs assetPath on its success path, so assetsSave may not exist when writeOneFile(assets.json) runs a couple lines below — this recursive mkdir prevents that ENOENT, and is idempotent for the normal path. No change requested.

const assetMasterFolderPath = path.join(assetsSave, ASSETS_FAILED_FILE);

await writeOneFile(path.join(assetsSave, ASSETS_SCHEMA_FILE), assetData);
Comment thread
chetan-contentstack marked this conversation as resolved.
Expand Down Expand Up @@ -833,6 +830,7 @@ const createAssets = async (packagePath: any, destination_stack_id: string, proj
err
)
await customLogger(projectId, destination_stack_id, 'error', message);
throw err;

@umesh-more-cstk umesh-more-cstk Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

blocker: Re-throwing here is the right intent (abort the sequential step chain so createEntry never runs on a half-written assets.json), but trace where the rejection ends up:

  • createAssets is awaited inside the Contentful branch of startTestMigration / startMigration (migration.service.ts L549 / L996), which have no outer try/catch.
  • The controller calls them fire-and-forget: const resp = migrationService.startTestMigration(req); res.status(200).json(resp); — no await, no .catch() (migration.controller.ts:27, and :40 for startMigration). asyncRouter only catches the controller's own rejection, not this detached promise.
  • There is no process.on('unhandledRejection') anywhere in api and no --unhandled-rejections flag in the start scripts.

So any FS failure the outer catch sees (this readFile, JSON.parse, mkdir, or a writeOneFile reject) now becomes an unhandled promise rejection. On Node's default mode (throw, v15+) that terminates the entire API process — killing all other in-flight migrations — and the user already received 200, so the UI never sees the failure. Previously the swallow-and-continue kept the process alive.

Suggested fix: don't let it propagate to the detached promise. Either handle it where the migration actually runs, e.g. in startTestMigration/startMigration wrap the step switch and mark the project failed + log (fail fast without crashing), or attach a .catch(err => { logger.error(err); /* mark failed */ }) at the fire-and-forget call in the controller, and/or register a global process.on('unhandledRejection', ...) as a safety net. customLogger already logs here, so the throw's job is just the abort — it needs a catcher upstream.

}
};

Expand Down
Loading