Skip to content

Commit 1a6633c

Browse files
benbuckbenbuck
committed
Better error messages
1 parent e6ade59 commit 1a6633c

File tree

12 files changed

+184
-87
lines changed

12 files changed

+184
-87
lines changed

‎TODO.md‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
- Add tray icon promotion (avoid moving to notification area overflow menu)
44
- Improve method for spy window selection
5-
- Specify which hotkey on registration failure
65
- Investigate VirusTotal results for installer
76
- Add auto-tray option: when created, when minimized, both
87
- Add reset button to settings dialog

‎src/ErrorContext.h‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Copyright 2020 Benbuck Nason
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#pragma once
16+
17+
// Standard library
18+
#include <string>
19+
20+
class ErrorContext
21+
{
22+
public:
23+
inline explicit ErrorContext(unsigned int errorId)
24+
: errorId_(errorId)
25+
{
26+
}
27+
28+
inline ErrorContext(unsigned int errorId, const std::string & errorString)
29+
: errorId_(errorId)
30+
, errorString_(errorString)
31+
{
32+
}
33+
34+
inline operator bool() const { return (errorId_ != 0) || !errorString_.empty(); }
35+
36+
inline unsigned int errorId() const { return errorId_; }
37+
38+
inline const std::string & errorString() const { return errorString_; }
39+
40+
private:
41+
ErrorContext() = delete;
42+
43+
unsigned int errorId_;
44+
std::string errorString_;
45+
};

‎src/Finestray.cpp‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ enum class HotkeyID
6363
};
6464

6565
LRESULT wndProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam);
66-
int start();
66+
ErrorContext start();
6767
void stop();
6868
bool windowShouldAutoTray(HWND hwnd);
6969
void onAddWindow(HWND hwnd);
@@ -134,7 +134,7 @@ int WINAPI wWinMain(_In_ HINSTANCE hinstance, _In_opt_ HINSTANCE prevHinstance,
134134
DEBUG_PRINTF("read settings from %s\n", settingsFile.c_str());
135135
} else {
136136
if (Settings::fileExists(settingsFile)) {
137-
errorMessage(IDS_ERROR_LOAD_SETTINGS);
137+
errorMessage(ErrorContext(IDS_ERROR_LOAD_SETTINGS, settingsFile));
138138
return IDS_ERROR_LOAD_SETTINGS;
139139
}
140140

@@ -161,10 +161,9 @@ int WINAPI wWinMain(_In_ HINSTANCE hinstance, _In_opt_ HINSTANCE prevHinstance,
161161
wc.hIconSm = hicon;
162162
ATOM atom = RegisterClassExA(&wc);
163163
if (!atom) {
164-
ERROR_PRINTF(
165-
"could not create window class, RegisterClassExA() failed: %s",
166-
StringUtility::lastErrorString().c_str());
167-
errorMessage(IDS_ERROR_REGISTER_WINDOW_CLASS);
164+
std::string lastErrorString = StringUtility::lastErrorString();
165+
ERROR_PRINTF("could not create window class, RegisterClassExA() failed: %s", lastErrorString.c_str());
166+
errorMessage(ErrorContext(IDS_ERROR_REGISTER_WINDOW_CLASS, lastErrorString));
168167
return IDS_ERROR_REGISTER_WINDOW_CLASS;
169168
}
170169

@@ -183,8 +182,9 @@ int WINAPI wWinMain(_In_ HINSTANCE hinstance, _In_opt_ HINSTANCE prevHinstance,
183182
nullptr // application data
184183
);
185184
if (!appWindow_) {
186-
ERROR_PRINTF("could not create window, CreateWindowA() failed: %s", StringUtility::lastErrorString().c_str());
187-
errorMessage(IDS_ERROR_CREATE_WINDOW);
185+
std::string lastErrorString = StringUtility::lastErrorString();
186+
ERROR_PRINTF("could not create window, CreateWindowA() failed: %s", lastErrorString.c_str());
187+
errorMessage(ErrorContext(IDS_ERROR_CREATE_WINDOW, lastErrorString));
188188
return IDS_ERROR_CREATE_WINDOW;
189189
}
190190

@@ -208,15 +208,16 @@ int WINAPI wWinMain(_In_ HINSTANCE hinstance, _In_opt_ HINSTANCE prevHinstance,
208208
0,
209209
WINEVENT_OUTOFCONTEXT));
210210
if (!minimizeEventHook) {
211+
std::string lastErrorString = StringUtility::lastErrorString();
211212
ERROR_PRINTF(
212213
"failed to hook minimize win event %#x, SetWinEventHook() failed: %s\n",
213214
(HWND)appWindow_,
214-
StringUtility::lastErrorString().c_str());
215-
errorMessage(IDS_ERROR_REGISTER_EVENTHOOK);
215+
lastErrorString.c_str());
216+
errorMessage(ErrorContext(IDS_ERROR_REGISTER_EVENTHOOK, lastErrorString));
216217
return IDS_ERROR_REGISTER_EVENTHOOK;
217218
}
218219

219-
int err = start();
220+
ErrorContext err = start();
220221
if (err) {
221222
errorMessage(err);
222223
settingsDialogWindow_ = SettingsDialog::create(appWindow_, settings_, onSettingsDialogComplete);
@@ -449,83 +450,83 @@ LRESULT wndProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
449450
return DefWindowProc(hwnd, uMsg, wParam, lParam);
450451
}
451452

452-
int start()
453+
ErrorContext start()
453454
{
454455
Log::start(settings_.logToFile_, APP_NAME ".log");
455456

456457
// register a hotkey that will be used to minimize windows
457458
UINT vkMinimize = VK_DOWN;
458459
UINT modifiersMinimize = MOD_ALT | MOD_CONTROL | MOD_SHIFT;
459460
if (!Hotkey::parse(settings_.hotkeyMinimize_, vkMinimize, modifiersMinimize)) {
460-
return IDS_ERROR_REGISTER_HOTKEY;
461+
return ErrorContext(IDS_ERROR_PARSE_HOTKEY, "minimize");
461462
}
462463
if (!vkMinimize || !modifiersMinimize) {
463464
INFO_PRINTF("no hotkey to minimize windows\n");
464465
} else {
465466
if (!hotkeyMinimize_.create((INT)HotkeyID::Minimize, appWindow_, vkMinimize, modifiersMinimize | MOD_NOREPEAT)) {
466-
return IDS_ERROR_REGISTER_HOTKEY;
467+
return ErrorContext(IDS_ERROR_REGISTER_HOTKEY, "minimize");
467468
}
468469
}
469470

470471
// register a hotkey that will be used to restore windows
471472
UINT vkRestore = VK_UP;
472473
UINT modifiersRestore = MOD_ALT | MOD_CONTROL | MOD_SHIFT;
473474
if (!Hotkey::parse(settings_.hotkeyRestore_, vkRestore, modifiersRestore)) {
474-
return IDS_ERROR_REGISTER_HOTKEY;
475+
return ErrorContext(IDS_ERROR_PARSE_HOTKEY, "restore");
475476
}
476477
if (!vkRestore || !modifiersRestore) {
477478
INFO_PRINTF("no hotkey to restore windows\n");
478479
} else {
479480
if (!hotkeyRestore_.create((INT)HotkeyID::Restore, appWindow_, vkRestore, modifiersRestore | MOD_NOREPEAT)) {
480-
return IDS_ERROR_REGISTER_HOTKEY;
481+
return ErrorContext(IDS_ERROR_REGISTER_HOTKEY, "restore");
481482
}
482483
}
483484

484485
// register a hotkey that will be used to restore all windows
485486
UINT vkRestoreAll = VK_LEFT;
486487
UINT modifiersRestoreAll = MOD_ALT | MOD_CONTROL | MOD_SHIFT;
487488
if (!Hotkey::parse(settings_.hotkeyRestoreAll_, vkRestoreAll, modifiersRestoreAll)) {
488-
return IDS_ERROR_REGISTER_HOTKEY;
489+
return ErrorContext(IDS_ERROR_PARSE_HOTKEY, "restore all");
489490
}
490491
if (!vkRestoreAll || !modifiersRestoreAll) {
491492
INFO_PRINTF("no hotkey to restore all windows\n");
492493
} else {
493494
if (!hotkeyRestoreAll_
494495
.create((INT)HotkeyID::RestoreAll, appWindow_, vkRestoreAll, modifiersRestoreAll | MOD_NOREPEAT)) {
495-
return IDS_ERROR_REGISTER_HOTKEY;
496+
return ErrorContext(IDS_ERROR_REGISTER_HOTKEY, "restore all");
496497
}
497498
}
498499

499500
// register a hotkey that will be used to show the context menu
500501
UINT vkMenu = VK_HOME;
501502
UINT modifiersMenu = MOD_ALT | MOD_CONTROL | MOD_SHIFT;
502503
if (!Hotkey::parse(settings_.hotkeyMenu_, vkMenu, modifiersMenu)) {
503-
return IDS_ERROR_REGISTER_HOTKEY;
504+
return ErrorContext(IDS_ERROR_PARSE_HOTKEY, "menu");
504505
}
505506
if (!vkMenu || !modifiersMenu) {
506507
INFO_PRINTF("no hotkey to Menu windows\n");
507508
} else {
508509
if (!hotkeyMenu_.create((INT)HotkeyID::Menu, appWindow_, vkMenu, modifiersMenu | MOD_NOREPEAT)) {
509-
return IDS_ERROR_REGISTER_HOTKEY;
510+
return ErrorContext(IDS_ERROR_REGISTER_HOTKEY, "menu");
510511
}
511512
}
512513

513514
// get modifiers that will be used to override auto-tray
514515
UINT vkOverride = 0;
515516
modifiersOverride_ = MOD_ALT | MOD_CONTROL | MOD_SHIFT;
516517
if (!Hotkey::parse(settings_.modifiersOverride_, vkOverride, modifiersOverride_)) {
517-
return IDS_ERROR_REGISTER_MODIFIER;
518+
return ErrorContext(IDS_ERROR_PARSE_MODIFIER, "override");
518519
}
519520
if (!modifiersOverride_) {
520521
INFO_PRINTF("no override modifiers\n");
521522
} else if (vkOverride || (modifiersOverride_ & ~(MOD_ALT | MOD_CONTROL | MOD_SHIFT))) {
522523
WARNING_PRINTF("invalid override modifiers\n");
523-
return IDS_ERROR_REGISTER_MODIFIER;
524+
return ErrorContext(IDS_ERROR_REGISTER_MODIFIER, "override");
524525
}
525526

526527
WindowList::start(appWindow_, settings_.pollInterval_, onAddWindow, onRemoveWindow, onChangeWindowTitle, onChangeVisibility);
527528

528-
return 0;
529+
return ErrorContext(0);
529530
}
530531

531532
void stop()
@@ -713,7 +714,7 @@ void onSettingsDialogComplete(bool success, const Settings & settings)
713714

714715
// restart to apply new settings
715716
stop();
716-
int err = start();
717+
ErrorContext err = start();
717718
if (err) {
718719
errorMessage(err);
719720
settingsDialogWindow_ = SettingsDialog::create(appWindow_, settings_, onSettingsDialogComplete);
@@ -722,7 +723,7 @@ void onSettingsDialogComplete(bool success, const Settings & settings)
722723
}
723724

724725
if (!settings_.writeToFile(settingsFile)) {
725-
errorMessage(IDS_ERROR_SAVE_SETTINGS);
726+
errorMessage(ErrorContext(IDS_ERROR_SAVE_SETTINGS, settingsFile));
726727
} else {
727728
DEBUG_PRINTF("wrote settings to %s\n", settingsFile.c_str());
728729
}

‎src/Finestray.rc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ BEGIN
5050
IDS_ERROR_INIT_COMMON_CONTROLS "Failed to initialize common controls"
5151
IDS_ERROR_REGISTER_WINDOW_CLASS "Error creating window class"
5252
IDS_ERROR_CREATE_WINDOW "Error creating window"
53+
IDS_ERROR_PARSE_HOTKEY "Couldn't parse hotkey"
5354
IDS_ERROR_REGISTER_HOTKEY "Couldn't register hotkey"
55+
IDS_ERROR_PARSE_MODIFIER "Couldn't parse modifier"
5456
IDS_ERROR_REGISTER_MODIFIER "Couldn't register modifier"
5557
IDS_ERROR_REGISTER_EVENTHOOK "Couldn't register event hook"
5658
IDS_ERROR_CREATE_TRAY_ICON "Couldn't create tray icon"

‎src/Helpers.cpp‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,29 @@ std::string getWindowClassName(HWND hwnd)
7878
void errorMessage(unsigned int id)
7979
{
8080
const std::string & err = getResourceString(id);
81+
8182
ERROR_PRINTF("%s\n", err.c_str());
8283
if (!MessageBoxA(nullptr, err.c_str(), APP_NAME, MB_OK | MB_ICONERROR)) {
8384
WARNING_PRINTF(
84-
"failed to display error message %u, MessageBoxA() failed\n",
85+
"failed to display error message %u, MessageBoxA() failed: %s\n",
8586
id,
8687
StringUtility::lastErrorString().c_str());
8788
}
8889
}
90+
91+
void errorMessage(const ErrorContext & errorContext)
92+
{
93+
std::string err = getResourceString(errorContext.errorId());
94+
95+
if (!errorContext.errorString().empty()) {
96+
err += ": " + errorContext.errorString();
97+
}
98+
99+
ERROR_PRINTF("%s\n", err.c_str());
100+
if (!MessageBoxA(nullptr, err.c_str(), APP_NAME, MB_OK | MB_ICONERROR)) {
101+
WARNING_PRINTF(
102+
"failed to display error message %u, MessageBoxA() failed: %s\n",
103+
errorContext.errorId(),
104+
StringUtility::lastErrorString().c_str());
105+
}
106+
}

‎src/Helpers.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
#pragma once
1616

17+
// App
18+
#include "ErrorContext.h"
19+
1720
// Windows
1821
#include <Windows.h>
1922

@@ -24,3 +27,4 @@ std::string getResourceString(unsigned int id);
2427
std::string getWindowText(HWND hwnd);
2528
std::string getWindowClassName(HWND hwnd);
2629
void errorMessage(unsigned int id);
30+
void errorMessage(const ErrorContext & errorContext);

‎src/Hotkey.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ std::string Hotkey::normalize(const std::string & hotkeyStr)
114114
}
115115
} else {
116116
if (!key.empty()) {
117-
WARNING_PRINTF("more than one key in hotkey, ignoring '%s'\n", token.c_str());
117+
ERROR_PRINTF("more than one key in hotkey, can't normalize '%s'\n", token.c_str());
118+
return StringUtility::join(tokens, " ");
118119
} else {
119120
// look for vkey string
120121
const auto & vkit = vkeyMap_.find(token);
@@ -123,11 +124,13 @@ std::string Hotkey::normalize(const std::string & hotkeyStr)
123124
} else {
124125
// look for key character
125126
if (token.length() != 1) {
126-
WARNING_PRINTF("unknown value in hotkey, ignoring '%s'\n", token.c_str());
127+
ERROR_PRINTF("unknown value in hotkey, can't normalize '%s'\n", token.c_str());
128+
return StringUtility::join(tokens, " ");
127129
} else {
128130
SHORT scan = VkKeyScanA(token[0]);
129131
if ((unsigned int)scan == 0xFFFF) {
130-
WARNING_PRINTF("unknown key in hotkey, ignoring '%s'\n", token.c_str());
132+
ERROR_PRINTF("unknown key in hotkey, can't normalize '%s'\n", token.c_str());
133+
return StringUtility::join(tokens, " ");
131134
} else {
132135
key = token;
133136
}

‎src/Path.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,10 @@ std::string getStartupDir()
102102

103103
std::string pathJoin(const std::string & path1, const std::string & path2)
104104
{
105+
size_t pathSize = std::min<size_t>(MAX_PATH, path1.size() + path2.size() + 2);
106+
105107
std::string path;
106-
path.resize(std::min<size_t>(MAX_PATH, path1.size() + path2.size() + 2));
108+
path.resize(pathSize);
107109

108110
LPSTR result = PathCombineA(&path[0], path1.c_str(), path2.c_str());
109111
if (!result) {
@@ -115,6 +117,8 @@ std::string pathJoin(const std::string & path1, const std::string & path2)
115117
return std::string();
116118
}
117119

120+
path.resize(pathSize - 1); // remove nul terminator
121+
118122
return path;
119123
}
120124

‎src/Resource.h‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,17 @@
3737
#define IDS_ERROR_INIT_COMMON_CONTROLS 302
3838
#define IDS_ERROR_REGISTER_WINDOW_CLASS 303
3939
#define IDS_ERROR_CREATE_WINDOW 304
40-
#define IDS_ERROR_REGISTER_HOTKEY 305
41-
#define IDS_ERROR_REGISTER_MODIFIER 306
42-
#define IDS_ERROR_REGISTER_EVENTHOOK 307
43-
#define IDS_ERROR_CREATE_TRAY_ICON 308
44-
#define IDS_ERROR_CREATE_MENU 309
45-
#define IDS_ERROR_COMMAND_LINE 310
46-
#define IDS_ERROR_CREATE_DIALOG 311
47-
#define IDS_ERROR_LOAD_SETTINGS 312
48-
#define IDS_ERROR_SAVE_SETTINGS 313
40+
#define IDS_ERROR_PARSE_HOTKEY 305
41+
#define IDS_ERROR_REGISTER_HOTKEY 306
42+
#define IDS_ERROR_PARSE_MODIFIER 307
43+
#define IDS_ERROR_REGISTER_MODIFIER 308
44+
#define IDS_ERROR_REGISTER_EVENTHOOK 309
45+
#define IDS_ERROR_CREATE_TRAY_ICON 310
46+
#define IDS_ERROR_CREATE_MENU 311
47+
#define IDS_ERROR_COMMAND_LINE 312
48+
#define IDS_ERROR_CREATE_DIALOG 313
49+
#define IDS_ERROR_LOAD_SETTINGS 314
50+
#define IDS_ERROR_SAVE_SETTINGS 315
4951

5052
// bitmaps
5153
#define IDB_APP 401

0 commit comments

Comments
 (0)