Prompt to reset settings when settings.json cannot be read #379

ryanofsky · 2021-07-08T19:17:41Z

Currently the GUI shows confusing error messages when settings.json can't be read or written on startup. This causes the unrecoverable read error described in bitcoin/bitcoin#21340 and write error described bitcoin/bitcoin#21974. Current error read message looks like:

This PR tries to clarify the error dialog, and adds an option to just clear the settings and reset them to default:

Additionally the PR also shows a slightly better error message when there is an error trying to write the settings file. This error probably should occur less frequently, but it is easy to improve, and it should be good to make the write error consistent with the read error. The new write error dialog looks like:

Fixes bitcoin/bitcoin#21340 Co-authored-by: Jarol Rodriguez <[email protected]>

ryanofsky · 2021-07-08T19:30:57Z

Note: Screenshots above are from linux and I think the button placement Qt chooses there is pretty strange. I think it's weird that the default button is on the left in the read error dialog, but the default button is on the right in the write error dialog. I also think it's weird that the show details toggle is between two other buttons that dismiss the dialog. None of this is very bad, but I'm hoping maybe it looks a little better on other platforms, since it makes probably doesn't make sense to hardcode a button order if conventions vary across platforms.

Also, the following patch is an easy way to test the PR, and it's what I used to make the screenshots:

--- a/src/util/settings.cpp
+++ b/src/util/settings.cpp
@@ -65,7 +65,7 @@ bool ReadSettings(const fs::path& path, std::map<std::string, SettingsValue>& va
     if (!file.is_open()) return true; // Ok for file not to exist.
 
     SettingsValue in;
-    if (!in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) {
+    if (1 | !in.read(std::string{std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()})) {
         errors.emplace_back(strprintf("Unable to parse settings file %s", path.string()));
         return false;
     }
@@ -102,7 +102,7 @@ bool WriteSettings(const fs::path& path,
     }
     fsbridge::ofstream file;
     file.open(path);
-    if (file.fail()) {
+    if (1 | file.fail()) {
         errors.emplace_back(strprintf("Error: Unable to open settings file %s for writing", path.string()));
         return false;
     }

ryanofsky · 2021-07-13T19:24:04Z

Updated cf47205 -> ebfd619 (pr/badset.1 -> pr/badset.2, compare) to fix appveyor error https://ci.appveyor.com/project/DrahtBot/bitcoin-core-gui/builds/39921403

jarolrod

ACK ebfd619

Tested functionality on macOS 11.3, Arch Linux and cross compiled to Windows 10. This is a nice change and makes these errors seem less scary. The nit on translator comments can be addressed in a follow-up, non-blocker.

Tested the error message would show up by applying this patch: #379 (comment)

Tested that I could abort and reset manually changing the settings file with the implemented buttons. Presentation of the read error was done by passing in a data directory that already had a settings file. The presentation of the write error was tested by passing in a new and empty data directory.

macOS 11.3

Read Error

Presentation	Expand Details

Write Error:

Presentation	Expand Details

Arch Linux

Read Error

Presentation	Expand Details

Write Error:

Presentation	Expand Details

Windows 10

Read Error

Presentation	Expand Details

Write Error:

Presentation	Expand Details

jarolrod · 2021-07-23T07:12:54Z

src/qt/bitcoin.cpp

+        InitError(Untranslated(strprintf("%s:\n%s\n", error.original, JoinErrors(errors))));
+
+        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort);
+        messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));


could add a translator comment:

diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index c72a5ecdd..bc9da65ab 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -161,6 +161,8 @@ static bool InitSettings() InitError(Untranslated(strprintf("%s:\n%s\n", error.original, JoinErrors(errors)))); QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort); + /*: Explanatory text shown on startup when the settings file cannot be read. + Prompts user to make a choice between resetting or aborting. */ messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?")); messagebox.setDetailedText(QString::fromStdString(JoinErrors(errors))); messagebox.setTextFormat(Qt::PlainText);

re: #379 (comment)

could add a translator comment:

Added, thanks!

I don't see the logic to actually reset the settings to defaults.

I don't see the logic to actually reset the settings to defaults.

It happens implicitly because if reading the settings file fails, the file that is written on line 181 will be empty.

But if it fails part way into reading the settings, won't the ones it already read remain?

re: #379 (comment)

But if it fails part way into reading the settings, won't the ones it already read remain?

To my surprise, it is actually technically possible for this to happen! And it needs to be fixed outside the GUI code so I filed a bug in the main repo bitcoin/bitcoin#22638. Practically speaking, the scenario where this would happen is nearly impossible and if it did happen, the effect would just be settings kept instead of discarded. So I think there's no need for this PR to be held up by the bug, but it is a good catch!

src/qt/bitcoin.cpp

shaavan

tACK ebfd619

Successfully compiled and tested on Ubuntu 20.04
I like the idea of elaborating the problem, and it is an improvement compared to the error message of the master branch. Because:

Read Error: Now, the user is aware of the issue and can either continue to GUI or abort and close the program.
Write Error: The error message had essential details to understand the problem and do something about it.

But I would like to suggest some elaborations to make the user’s work more straightforward if they face this issue.

Some recommended further elaborations:

In case of an error in writing the file or failure in overwriting, then.
- If settings.json file:
  - does not exist: create a new file with default values
  - exists: try deleting the file and make a new file with default values
- Abort
In case of not able to delete the file (above solution):
- Abort

Talkless · 2021-07-28T18:45:13Z

I've tried reproducing using permissions like this:

sudo chown root:root /tmp/datadir/regtest/settings.json
sudo chmod a-rwx  /tmp/datadir/regtest/settings.json

bitcoin-qt just silently (both master and PR) overwritten that file after experiencing EACCES (Permission denied):

$ strace -efile src/qt/bitcoin-qt -regtest -datadir=/tmp/datadir 2>&1 | fgrep settings.json
openat(AT_FDCWD, "/tmp/datadir/regtest/settings.json", O_RDONLY) = -1 EACCES (Permission denied)
openat(AT_FDCWD, "/tmp/datadir/regtest/settings.json.tmp", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 9
rename("/tmp/datadir/regtest/settings.json.tmp", "/tmp/datadir/regtest/settings.json") = 0

(it's probably allowed to rename because process is still owner of the directory).

Not saying it's this PR's issue, just interesting find...

Talkless · 2021-07-28T19:09:01Z

I've created AppArmor profile /etc/apparmor.d/bitcoin that attaches to the built binaries (master and pr) and allows (almost) everything, except access to the settings.json:

profile bitcoin /home/vincas/code/bitcoin/379-bitcoin-core-gui.git/_build_{master,pr}/src/qt/bitcoin-qt {

    /{,**} rwmklux, # allow reading/writing/locking/mmaping/linking/executing... everything

    # allow various other kernel api stuff:
    capability,
    network,
    dbus,
    ptrace,
    signal,
    unix,

    # ..except this one
    audit deny /tmp/datadir/regtest/settings.json rwmkl,
}

I do get dialog Failed renaming settings file... but the fact that reading has failed was "skipped":

$ strace -efile src/qt/bitcoin-qt -regtest -datadir=/tmp/datadir 2>&1 | fgrep settings.json
openat(AT_FDCWD, "/tmp/datadir/regtest/settings.json", O_RDONLY) = -1 EACCES (Permission denied)
openat(AT_FDCWD, "/tmp/datadir/regtest/settings.json.tmp", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 9
rename("/tmp/datadir/regtest/settings.json.tmp", "/tmp/datadir/regtest/settings.json") = -1 EACCES (Permission denied)
- Failed renaming settings file /tmp/datadir/regtest/settings.json.tmp to /tmp/datadir/regtest/settings.json

audit.log shows that AppArmor denied reading and then writing:

type=AVC msg=audit(1627499280.531:631): apparmor="DENIED" operation="open" profile="bitcoin" name="/tmp/datadir/regtest/settings.json" pid=39084 comm="bitcoin-qt" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000FSUID="vincas" OUID="vincas"
type=AVC msg=audit(1627499280.531:632): apparmor="DENIED" operation="rename_dest" profile="bitcoin" name="/tmp/datadir/regtest/settings.json" pid=39084 comm="bitcoin-qt" requested_mask="wc" denied_mask="wc" fsuid=1000 ouid=1000FSUID="vincas" OUID="vincas"

ryanofsky

Thanks for the reviews and testing!

Updated ebfd619 -> 1ee6d0b (pr/badset.2 -> pr/badset.3, compare) just adding jarolrod's suggested translator comments verbatim.

This PR may be close to ready for merge.

re: #379 (review)

Some recommended further elaborations:

Thanks, I think I basically agree with the suggestions. Would encouraging filing new issues if there are specific problems that aren't being addressed. This PR clarifies the write error somewhat, but after this PR there are two more issues that need to be resolved on settings write errors:

bitcoin/bitcoin#21974 - when settings file doesn't exist and writing a new one fails, it can be indicate a problem with the datadir, like maybe it was set to an external location that is no longer accessible, so a more convenient option to choose a new datadir could be provided
bitcoin/bitcoin#22571 - valid but inaccessible settings.json files could be overwritten in some cases due to read access errors

re: #379 (comment)

I've tried reproducing using permissions like this:
[...]

re: #379 (comment)

I've created AppArmor profile /etc/apparmor.d/bitcoin that attaches to the built binaries (master and pr) and allows (almost) everything, except access to the settings.json:
[...]

Great find! Both of these posts are describing the same bug and I filed a new issue bitcoin/bitcoin#22571. The fix needs to stop ignoring EACCES (Permission denied) and other read errors, instead of treating them the same as ENOENT (No such file or directory)

ryanofsky · 2021-07-29T22:40:22Z

src/qt/bitcoin.cpp

+        InitError(Untranslated(strprintf("%s:\n%s\n", error.original, JoinErrors(errors))));
+
+        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort);
+        messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));


re: #379 (comment)

could add a translator comment:

Added, thanks!

src/qt/bitcoin.cpp

luke-jr · 2021-07-30T20:52:01Z

src/qt/bitcoin.cpp

+        InitError(Untranslated(strprintf("%s:\n%s\n", error.original, JoinErrors(errors))));
+
+        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort);
+        messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));


I don't see the logic to actually reset the settings to defaults.

jarolrod

ACK 1ee6d0b

Changes since my last review are only translator comments, thanks for adding them 🥃

hebasto · 2021-08-05T11:42:50Z

Concept ACK.

hebasto · 2021-08-05T12:30:20Z

src/qt/bitcoin.cpp

+        /*: Explanatory text shown on startup when the settings file cannot be read.
+            Prompts user to make a choice between resetting or aborting. */
+        messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));
+        messagebox.setDetailedText(QString::fromStdString(JoinErrors(errors)));


Usually, we use a detailed text to show untranslated error messages to users for convenient googling and referencing (see bitcoin/bitcoin#16224). Now it only contains a list of errors. Could an untranslated error message be added here as well?

re: #379 (comment)

Usually, we use a detailed text to show untranslated error messages to users for convenient googling and referencing (see bitcoin/bitcoin#16224). Now it only contains a list of errors. Could an untranslated error message be added here as well?

Just to clarify what's being discussed, errors is untranslated, and the error messages are self contained, so this dialog is consistent with other dialogs in using translated text in the main section, and untranslated text in the details section. The inconsistency is just in the prefixing.

On the prefixing, I do think it would be reasonable for a followup PR to append BitcoinGUI::tr("Original message:") + "\n" + QString::fromStdString(error.original) if error.original != error.translated to be a consistent with ThreadSafeMessageBox but I think it would be better to do it in a separate PR because:

I think this logic should go in a common util function so it is not duplicated. Trying to do this and refactor the ThreadSafeMessageBox code would expand the scope of this PR and complicate it when I think this PR is already a strict improvement and it's been reviewed and tested by people.

In this specific place, just because of the fact that ReadSettings errors are fully self-describing, I think adding the extra text would actually make the UX slightly worse (more repetitive and confusing) so more discussion in a followup PR about how to have consistency and clarity in these dialogs at same time could be a good idea.

ryanofsky

Thanks for reviews! There's good discussion from luke and hebasto about issues that would be good to follow up on (#379 (comment), #379 (comment)), but to me it looks like with the testing and review this PR has had so far, it could be ready for merge.

I'd like this PR to remain simple and focused on the basic UX improvement of displaying settings.json read errors. I'd like future PRs to address settings write errors, settings read implementation bugs, and better distinctions between untranslated "details" and untranslated "original messages" as other improvements that can build on top of this improvement.

ryanofsky · 2021-08-05T18:47:46Z

src/qt/bitcoin.cpp

+        /*: Explanatory text shown on startup when the settings file cannot be read.
+            Prompts user to make a choice between resetting or aborting. */
+        messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));
+        messagebox.setDetailedText(QString::fromStdString(JoinErrors(errors)));


re: #379 (comment)

Usually, we use a detailed text to show untranslated error messages to users for convenient googling and referencing (see bitcoin/bitcoin#16224). Now it only contains a list of errors. Could an untranslated error message be added here as well?

Just to clarify what's being discussed, errors is untranslated, and the error messages are self contained, so this dialog is consistent with other dialogs in using translated text in the main section, and untranslated text in the details section. The inconsistency is just in the prefixing.

On the prefixing, I do think it would be reasonable for a followup PR to append BitcoinGUI::tr("Original message:") + "\n" + QString::fromStdString(error.original) if error.original != error.translated to be a consistent with ThreadSafeMessageBox but I think it would be better to do it in a separate PR because:

I think this logic should go in a common util function so it is not duplicated. Trying to do this and refactor the ThreadSafeMessageBox code would expand the scope of this PR and complicate it when I think this PR is already a strict improvement and it's been reviewed and tested by people.

In this specific place, just because of the fact that ReadSettings errors are fully self-describing, I think adding the extra text would actually make the UX slightly worse (more repetitive and confusing) so more discussion in a followup PR about how to have consistency and clarity in these dialogs at same time could be a good idea.

ryanofsky · 2021-08-05T19:18:52Z

src/qt/bitcoin.cpp

+        InitError(Untranslated(strprintf("%s:\n%s\n", error.original, JoinErrors(errors))));
+
+        QMessageBox messagebox(QMessageBox::Critical, PACKAGE_NAME, QString::fromStdString(strprintf("%s.", error.translated)), QMessageBox::Reset | QMessageBox::Abort);
+        messagebox.setInformativeText(QObject::tr("Do you want to reset settings to default values, or to abort without making changes?"));


re: #379 (comment)

But if it fails part way into reading the settings, won't the ones it already read remain?

To my surprise, it is actually technically possible for this to happen! And it needs to be fixed outside the GUI code so I filed a bug in the main repo bitcoin/bitcoin#22638. Practically speaking, the scenario where this would happen is nearly impossible and if it did happen, the effect would just be settings kept instead of discarded. So I think there's no need for this PR to be held up by the bug, but it is a good catch!

ghost · 2021-08-05T20:39:43Z

Concept ACK. Will test this weekend.

Zero-1729

ACK 1ee6d0b

hebasto

ACK 1ee6d0b, tested on Linux Mint 20.2 (Qt 5.12.8).

hebasto · 2021-08-06T19:13:18Z

src/qt/bitcoin.cpp

+        /*: Explanatory text shown on startup when the settings file could not be written.
+            Prompts user to check that we have the ability to write to the file.
+            Explains that the user has the option of running without a settings file.*/
+        messagebox.setInformativeText(QObject::tr("A fatal error occured. Check that settings file is writable, or try running with -nosettings."));


typo: occured ==> occurred

Addressed in bitcoin/bitcoin#22653.

…on cannot be read 1ee6d0b gui: Prompt to reset settings when settings.json cannot be read (Russell Yanofsky) Pull request description: Currently the GUI shows confusing error messages when `settings.json` can't be read or written on startup. This causes the unrecoverable read error described in bitcoin#21340 and write error described bitcoin#21974. Current error read message looks like: ![current](https://user-images.githubusercontent.com/7133040/124977362-638ffc80-dffe-11eb-9edd-89135a9bc602.png) This PR tries to clarify the error dialog, and adds an option to just clear the settings and reset them to default: ![new-read-error](https://user-images.githubusercontent.com/7133040/124977636-b669b400-dffe-11eb-8d35-02eda95f48c0.png) ![new-read-details](https://user-images.githubusercontent.com/7133040/124977644-bb2e6800-dffe-11eb-9209-11c1c3d7be40.png) Additionally the PR also shows a slightly better error message when there is an error trying to write the settings file. This error probably should occur less frequently, but it is easy to improve, and it should be good to make the write error consistent with the read error. The new write error dialog looks like: ![new-write-error](https://user-images.githubusercontent.com/7133040/124978016-3bed6400-dfff-11eb-9d79-9b2e9bbc4369.png) ![new-write-details](https://user-images.githubusercontent.com/7133040/124978025-3db72780-dfff-11eb-8df5-741f75a402d9.png) ACKs for top commit: jarolrod: ACK 1ee6d0b Zero-1729: ACK 1ee6d0b hebasto: ACK 1ee6d0b, tested on Linux Mint 20.2 (Qt 5.12.8). Tree-SHA512: fb57a0a0d032e3f8219fff49a4de69b4c962bf0b448544ccf9d8d4d45c5bd209e23653d4f13300b9e534b9c03de159498bef1658e95defe3ab6a8ecac57d592c

ghost · 2021-08-07T20:38:37Z

Tested on Windows with src/util/settings.cpp from bitcoin/bitcoin#22591

Post merge tACK 1ee6d0b

nit:

-Settings file could not be read
+Failed to read settings file

-Do you want to reset settings to default values, or to abort without making any changes?
+Do you want to reset settings to default values, or abort without making any changes?

nit:

DetailedText can be better if it was something like this:

File: E:\regtest\settings.json
Error: Please check permissions

If DetailedText is changed above, can follow the same format for write error as well.

bb56486 refactor: Reuse MakeUnorderedList where possible (Hennadii Stepanov) 77a90f0 refactor: Move MakeUnorderedList into util/string.h to make it reusable (Hennadii Stepanov) 6a5ccd6 scripted-diff: Rename JoinErrors in more general MakeUnorderedList (Hennadii Stepanov) Pull request description: A nice `JoinErrors` utility function was introduced in #379 by Russell Yanofsky. This PR renames this function and re-uses it across the code base. ACKs for top commit: Zero-1729: Concept ACK bb56486 theStack: Code-review ACK bb56486 Talkless: utACK bb56486 ryanofsky: Code review ACK bb56486. Nice deduping, thanks for this! Tree-SHA512: 6bdbfa61f2ffa69e075f46b733f247c6d5b8486779a1dac064285a199a4bb8bc5ef44eaee37086305646b5c88eb6a11990883219a4a9140a5117ee21ed529bb9

… is unreadable 2b07126 error if settings.json exists, but is unreadable (Tyler Chambers) Pull request description: If settings.json exists, but is unreadable, we should error instead of overwriting. Fixes #22571 ACKs for top commit: Zero-1729: tACK 2b07126 ShaMan239: tACK 2b07126 prayank23: tACK bitcoin/bitcoin@2b07126 ryanofsky: Code review ACK 2b07126. Thanks for the fix! Note that PR #379 will change the appearance of dialogs shown in screenshots above. So it could be interesting to test the two PRs together (but current testing seems more than sufficient) theStack: ACK 2b07126 📁 Tree-SHA512: 6f7f96ce8a13213d0335198a2245d127264495c877105058d1503252435915b332a6e55068ac21088f4c0c017d564689f4956213328d5bdee81d73711efc5511

…adable 2b07126 error if settings.json exists, but is unreadable (Tyler Chambers) Pull request description: If settings.json exists, but is unreadable, we should error instead of overwriting. Fixes bitcoin#22571 ACKs for top commit: Zero-1729: tACK 2b07126 ShaMan239: tACK 2b07126 prayank23: tACK bitcoin@2b07126 ryanofsky: Code review ACK 2b07126. Thanks for the fix! Note that PR bitcoin-core/gui#379 will change the appearance of dialogs shown in screenshots above. So it could be interesting to test the two PRs together (but current testing seems more than sufficient) theStack: ACK 2b07126 📁 Tree-SHA512: 6f7f96ce8a13213d0335198a2245d127264495c877105058d1503252435915b332a6e55068ac21088f4c0c017d564689f4956213328d5bdee81d73711efc5511

gui: Prompt to reset settings when settings.json cannot be read

1ee6d0b

Fixes bitcoin/bitcoin#21340 Co-authored-by: Jarol Rodriguez <[email protected]>

This was referenced Jul 8, 2021

"Unable to parse settings.json" at startup after disk was full bitcoin/bitcoin#21340

Closed

windows: settings.json issue bitcoin/bitcoin#21974

Closed

ryanofsky force-pushed the pr/badset branch from cf47205 to ebfd619 Compare July 13, 2021 19:21

hebasto added the UX All about "how to get things done" label Jul 14, 2021

jarolrod reviewed Jul 23, 2021

View reviewed changes

shaavan reviewed Jul 27, 2021

View reviewed changes

ryanofsky mentioned this pull request Jul 28, 2021

settings.json overwritten if missing read permissions bitcoin/bitcoin#22571

Closed

ryanofsky force-pushed the pr/badset branch from ebfd619 to 1ee6d0b Compare July 29, 2021 23:08

ryanofsky commented Jul 29, 2021

View reviewed changes

luke-jr suggested changes Jul 30, 2021

View reviewed changes

jarolrod reviewed Aug 3, 2021

View reviewed changes

hebasto reviewed Aug 5, 2021

View reviewed changes

ryanofsky mentioned this pull request Aug 5, 2021

Bug: ArgsManager::ReadSettingsFile can return false even when it does load settings bitcoin/bitcoin#22638

Closed

ryanofsky commented Aug 5, 2021

View reviewed changes

Zero-1729 mentioned this pull request Aug 5, 2021

Util: error if settings json exists, but is unreadable bitcoin/bitcoin#22591

Merged

Zero-1729 reviewed Aug 5, 2021

View reviewed changes

hebasto approved these changes Aug 6, 2021

View reviewed changes

hebasto merged commit 7ebc4c6 into bitcoin-core:master Aug 6, 2021

hebasto mentioned this pull request Aug 6, 2021

refactor: Rename JoinErrors and re-use it bitcoin/bitcoin#22653

Merged

hebasto reviewed Aug 6, 2021

View reviewed changes

bitcoin-core locked as resolved and limited conversation to collaborators Sep 4, 2022

Prompt to reset settings when settings.json cannot be read #379

Prompt to reset settings when settings.json cannot be read #379

Uh oh!

Conversation

ryanofsky commented Jul 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky commented Jul 8, 2021

Uh oh!

ryanofsky commented Jul 13, 2021

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

macOS 11.3

Arch Linux

Windows 10

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shaavan left a comment

Choose a reason for hiding this comment

Uh oh!

Talkless commented Jul 28, 2021

Uh oh!

Talkless commented Jul 28, 2021

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto commented Aug 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 5, 2021

Uh oh!

Zero-1729 left a comment

Choose a reason for hiding this comment

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 7, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

ryanofsky commented Jul 8, 2021 •

edited

Loading

ryanofsky left a comment •

edited

Loading

ghost commented Aug 7, 2021 •

edited by ghost

Loading