Skip to content

fix(instance): remove new directory when failing to create instance#1321

Open
icgnos wants to merge 1 commit intoUNIkeEN:mainfrom
icgnos:error-dir
Open

fix(instance): remove new directory when failing to create instance#1321
icgnos wants to merge 1 commit intoUNIkeEN:mainfrom
icgnos:error-dir

Conversation

@icgnos
Copy link
Collaborator

@icgnos icgnos commented Jan 20, 2026 •
edited
Loading

Checklist

  • Changes have been tested locally and work as expected.
  • All tests in workflows pass successfully.
  • Documentation has been updated if necessary.
  • Code formatting and commit messages align with the project's conventions.
  • Comments have been added for any complex logic or functionality if possible.

This PR is a ..

  • 🆕 New feature
  • 🐞 Bug fix
  • 🛠 Refactoring
  • ⚡️ Performance improvement
  • 🌐 Internationalization
  • 📄 Documentation improvement
  • 🎨 Code style optimization
  • ❓ Other (Please specify below)

Related Issues

Description

  • 除了名称冲突导致的失败，都会删除新的文件夹。

Additional Context

  • 如果是用户修改了资源索引文件导致下载失败，那么仍然无法解决。不过个人认为不用考虑这种情况

@icgnos icgnos changed the title fix(instance): remove new directory when failing to create instance (#1310, #1105) fix(instance): remove new directory when failing to create instance Jan 20, 2026
@Reqwey
Copy link
Contributor

Reqwey commented Jan 20, 2026 •
edited
Loading

发现你的代码里面在疯狂地添加

.map_err(|_| {
          let _ = fs::remove_dir_all(&version_path);

其实前面的时候这个目录都还没有创建。另外如此重复的写相同的代码十分地不优雅。

我的建议是，可以做成全局的错误捕捉，如把 create_instance 中的整个核心逻辑放到 helpers 里面作为一个新的工具函数，在 create_instance 里面调用这个函数并且针对它的错误返回值进行统一的删除目录处理。

UNIkeEN
UNIkeEN requested changes Jan 21, 2026
View reviewed changes
Copy link
Owner

@UNIkeEN UNIkeEN left a comment

Choose a reason for hiding this comment

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

另外，更重要的问题在于，下载任务创建但失败后，是否应该删除已经下载的部分文件（和文件夹），这个需要提请 UX 讨论，所以 issue 可能无法通过此 PR 关闭

src-tauri/src/instance/commands.rs Outdated
Copy link
Owner

@UNIkeEN UNIkeEN Jan 21, 2026

Choose a reason for hiding this comment

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

请通过断点检查，此行之前 version_path 是否已经被创建。在这之前的 remove_dir_all 可能是无效且无必要的

Copy link
Collaborator Author

@icgnos icgnos Jan 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

请通过断点检查，此行之前 version_path 是否已经被创建。在这之前的 remove_dir_all 可能是无效且无必要的

经测试在980行已被创建

src-tauri/src/instance/commands.rs Outdated
fs::write(options_path, format!("lang:{}\n", lang_code))
.map_err(|_| InstanceError::FileCreationFailed)?;
fs::write(options_path, format!("lang:{}\n", lang_code)).map_err(|_| {
let _ = fs::remove_dir_all(&version_path);
Copy link
Owner

@UNIkeEN UNIkeEN Jan 21, 2026

Choose a reason for hiding this comment

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

这里原来有 map_err，但应改为静默处理。自动跳过语言失败不应该将整个实例过程以错误终止

静默处理即 let _ = ...

Copy link
Owner

@UNIkeEN UNIkeEN Jan 21, 2026

Choose a reason for hiding this comment

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

静默处理即 let _ = ...

静默处理可以 log error

@icgnos icgnos requested a review from UNIkeEN January 22, 2026 02:33
@icgnos
Copy link
Collaborator Author

icgnos commented Jan 23, 2026

下载任务创建但失败后，是否应该删除已经下载的部分文件（和文件夹）

似乎下载任务创建之后就不会失败了，如果网络中断任务只会暂停，网络恢复后会继续下载。如果玩家手动取消任务，则会保留已下载的部分文件

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

Reviewers

@UNIkeEN UNIkeEN Awaiting requested review from UNIkeEN

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] 整合包倒入失败时应删除对应文件夹 [Bug] 实例下载任务创建失败后无法重试

3 participants

@icgnos @Reqwey @UNIkeEN