Page MenuHomePhorge
Differential D104

Fix crash when upload failed
ClosedPublic
Actions

Authored by nannanko on Jul 6 2024, 5:10 PM.

Details

Reviewers
tusooa
Group Reviewers
O1: the Kazv Project
Maniphest Tasks
T106: App will crash if uploading file failed
Commits
rK4bb140bd8261: Fix crash when upload failed
Summary

When response isn't an valid json, nlohmann::json::parse() will cause a crash. This fixed it.

Type: fix

Test Plan

Verify that kazv doesn't crash when upload failed.

Diff Detail

Repository
rK kazv
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

nannanko created this revision.Jul 6 2024, 5:10 PM
Owners added a reviewer: O1: the Kazv Project.Jul 6 2024, 5:10 PM
nannanko added a task: T106: App will crash if uploading file failed.Jul 6 2024, 5:11 PM
nannanko added a comment.Jul 6 2024, 5:14 PM

There is currently a lack of unit tests, and I'm not sure how to write them at the moment. I will add all related tests for uploading and downloading soon.

Harbormaster completed remote builds in B148: Diff 258.Jul 6 2024, 5:26 PM
nannanko requested review of this revision.Jul 6 2024, 5:26 PM
tusooa added inline comments.Jul 6 2024, 5:41 PM
src/kazv-io-job.cpp
299–300

Why not use try-catch?

Also Better to log it.

299–300

Don't assume the json body is in valid format. There are plenty of things that can go wrong.

tusooa requested changes to this revision.Jul 6 2024, 5:41 PM
This revision now requires changes to proceed.Jul 6 2024, 5:41 PM
nannanko updated this revision to Diff 280.Jul 12 2024, 6:50 AM
  • Use try-catch for exception
nannanko marked 2 inline comments as done.Jul 12 2024, 6:52 AM
Harbormaster completed remote builds in B166: Diff 280.Jul 12 2024, 7:07 AM
tusooa requested changes to this revision.Jul 12 2024, 7:08 AM
tusooa added inline comments.
src/kazv-io-job.cpp
300–315

Note, your original code doesn't create a default-constructed json. This creates an empty json array instead.

305

should be qCWarning(kazvLog)

309
310
313

should be qCWarning(kazvLog)

src/l10n/en/100-ui.ftl
215

missing translations for cmn-Hans

This revision now requires changes to proceed.Jul 12 2024, 7:08 AM
nannanko updated this revision to Diff 289.Jul 13 2024, 5:52 AM
  • Add cmn-Hans translations
  • Use qCWarning instead of qDebug
  • Check json format, fix a template call
nannanko marked 6 inline comments as done.Jul 13 2024, 5:53 AM
Harbormaster completed remote builds in B170: Diff 289.Jul 13 2024, 6:08 AM
tusooa accepted this revision.Jul 13 2024, 9:08 AM
This revision is now accepted and ready to land.Jul 13 2024, 9:08 AM
Closed by commit rK4bb140bd8261: Fix crash when upload failed. · Explain WhyJul 13 2024, 9:10 AM
This revision was automatically updated to reflect the committed changes.
tusooa added a commit: rK4bb140bd8261: Fix crash when upload failed.

Revision Contents
Changeset List

Diff 295

View Options

src/contents/ui/KazvIOMenu.qml

Loading...
View Options

src/kazv-io-job.hpp

Loading...
View Options

src/kazv-io-job.cpp

Loading...
View Options

src/l10n/cmn-Hans/100-ui.ftl

Loading...
View Options

src/l10n/en/100-ui.ftl

Loading...