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.

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

fix: TypeError occurs during export #9481

Merged
merged 2 commits into from
Dec 12, 2024

Conversation

miya
Copy link
Member

@miya miya commented Dec 12, 2024

Task

  • #158989 POST /_api/v3/export 時にサーバー側でエラーが出る
  • #159015 export 後に次の export ができない

経緯

  1. https://github.com/weseek/growi/pull/9361/files#diff-24c321cd621aaa24219022f980552d9f29902d40749092511a496e46ceeff06d
    • stream/promises package の pipeline を利用
  2. https://github.com/weseek/growi/pull/9455/files#diff-24c321cd621aaa24219022f980552d9f29902d40749092511a496e46ceeff06d
    • finished を用いて完了待ちを実装
  3. 本PR

@miya miya requested a review from yuki-takei December 12, 2024 07:14
@miya miya self-assigned this Dec 12, 2024
Copy link

changeset-bot bot commented Dec 12, 2024

⚠️ No Changeset found

Latest commit: 6920dba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@@ -349,13 +349,12 @@ class ExportService {

const output = fs.createWriteStream(zipFile);

// pipe archive data to the file
const stream = pipeline(archive, output);
Copy link
Member Author

Choose a reason for hiding this comment

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

https://redmine.weseek.co.jp/issues/158989 の修正

以下のエラー内容通り、await して promise instance を渡さないように修正

@growi/app:dev:   Unhandled Rejection: Promise: Promise {
@growi/app:dev:     <rejected> TypeError: The "stream" argument must be an instance of ReadableStream, WritableStream, or Stream. Received an instance of Promise
@growi/app:dev:         at eos (node:internal/streams/end-of-stream:74:11)
@growi/app:dev:         at node:internal/streams/end-of-stream:314:21
@growi/app:dev:         at new Promise (<anonymous>)
@growi/app:dev:         at finished (node:internal/streams/end-of-stream:313:10)
@growi/app:dev:         at ExportService.zipFiles (/workspace/growi/apps/app/src/server/service/export.js:358:11)
@growi/app:dev:         at ExportService.exportCollectionsToZippedJson (/workspace/growi/apps/app/src/server/service/export.js:229:32)
@growi/app:dev:         at processTicksAndRejections (node:internal/process/task_queues:95:5)
@growi/app:dev:         at async ExportService.export (/workspace/growi/apps/app/src/server/service/export.js:252:21) {
@growi/app:dev:       code: 'ERR_INVALID_ARG_TYPE'
@growi/app:dev:     }

await finished(stream);

// pipe archive data to the file
await pipeline(archive, output);

Copy link
Member Author

@miya miya Dec 12, 2024

Choose a reason for hiding this comment

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

https://redmine.weseek.co.jp/issues/159015 の修正

バグの原因

  1. POST /_api/v3/export を叩き zip ファイルを作成する。exportService の以下のクラス変数に値が入る。
    this.currentProgressingStatus = new ExportProgressingStatus(collections);
  2. しかし try ブロックの中で実行されている処理が原因で以下の finaly ブロックが実行されない (バグの原因1で入ったデータが残り続ける)
    this.currentProgressingStatus = null;
  3. クライアントは GET /_api/v3/export/status を叩き export 中の処理が無いかをチェックしている。export 中の 処理がある場合は export ボタンが disabled になる。export 中の処理があるか否かは exportService の getStatus() の中で this.currentProgressingStatus が null か否かをみている。バグの原因1の時点で残り続けたデータが存在することで export 中の判定がされてしまってる。

finaly ブロックが実行されない原因

archive.finalize() が実行される前に await pipeline(archive, output) が実行されていたことによって archive 側の処理の終了を待つ前に pipeline() 側が完了待ちになってしまったことでこのメソッド自体が終了しなかったことが原因(158989 の修正によって await を追加したため)。

そのため archive.finalize() が先に実行されるように修正した。pipeline を await することによって stream の完了待ちができるため await finished(stream) は不要になったので削除した。

Copy link
Member

Choose a reason for hiding this comment

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

Claude に説明補足させた

image

image

await finished(stream);

// pipe archive data to the file
await pipeline(archive, output);

Copy link
Member

Choose a reason for hiding this comment

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

Claude に説明補足させた

image

image

mergify bot added a commit that referenced this pull request Dec 12, 2024
@mergify mergify bot merged commit 9ee712c into master Dec 12, 2024
24 of 28 checks passed
@mergify mergify bot deleted the fix/159007-typeerror-occurs-during-export branch December 12, 2024 16:11
@github-actions github-actions bot mentioned this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants