Propagate 5xx server error to user + reload button. Closes #5

This commit is contained in:
relikd
2019-07-06 13:27:00 +02:00
parent 29a48384c7
commit 8dc95dda63
10 changed files with 88 additions and 34 deletions

View File

@@ -6,9 +6,16 @@ and this project does NOT adhere to [Semantic Versioning](https://semver.org/spe
## [Unreleased] ## [Unreleased]
### Added
- Show users any 5xx server error response and extracted failure reason
- 5xx server errors have a reload button which will initiate a new download with the same URL
- Adding feed: Cmd+R will reload the same URL
- Settings, Feeds: Cmd+R will reload the data source
### Fixed ### Fixed
- Changed error message text when user cancels creation of new feed item - Changed error message text when user cancels creation of new feed item
- Comparing existing articles with nonexistent guid and link - Comparing existing articles with nonexistent guid and link
- Adding feed: If URLs can't be resolved in the first run (5xx error), try a second time. E.g., 'Done' click (issue: #5)
### Changed ### Changed
- Interface builder files replaced with code equivalent - Interface builder files replaced with code equivalent

View File

@@ -187,6 +187,7 @@ static NSEventModifierFlags fnKeyFlags = NSEventModifierFlagShift | NSEventModif
case 'a': if ([self sendAction:@selector(selectAll:) to:nil from:self]) return; break; case 'a': if ([self sendAction:@selector(selectAll:) to:nil from:self]) return; break;
case 'q': if ([self sendAction:@selector(performClose:) to:nil from:self]) return; break; case 'q': if ([self sendAction:@selector(performClose:) to:nil from:self]) return; break;
case 'w': if ([self sendAction:@selector(performClose:) to:nil from:self]) return; break; case 'w': if ([self sendAction:@selector(performClose:) to:nil from:self]) return; break;
case 'r': if ([self sendAction:@selector(reloadData) to:nil from:self]) return; break;
#pragma clang diagnostic push #pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wundeclared-selector" #pragma clang diagnostic ignored "-Wundeclared-selector"
case 'z': if ([self sendAction:@selector(undo:) to:nil from:self]) return; break; case 'z': if ([self sendAction:@selector(undo:) to:nil from:self]) return; break;

View File

@@ -201,12 +201,32 @@ static BOOL _nextUpdateIsForced = NO;
+ (void)asyncRequest:(NSURLRequest*)request block:(nonnull void(^)(NSData * _Nullable data, NSError * _Nullable error, NSHTTPURLResponse *response))block { + (void)asyncRequest:(NSURLRequest*)request block:(nonnull void(^)(NSData * _Nullable data, NSError * _Nullable error, NSHTTPURLResponse *response))block {
[[[self nonCachingSession] dataTaskWithRequest:request completionHandler:^(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error) { [[[self nonCachingSession] dataTaskWithRequest:request completionHandler:^(NSData * _Nullable data, NSURLResponse * _Nullable response, NSError * _Nullable error) {
NSHTTPURLResponse* httpResponse = (NSHTTPURLResponse*)response; NSHTTPURLResponse* httpResponse = (NSHTTPURLResponse*)response;
if (error || [httpResponse statusCode] == 304) NSInteger status = [httpResponse statusCode];
if (error || status == 304) { // 304 Not Modified
data = nil; data = nil;
} else if (status >= 500 && status < 600) { // 5xx Server Error
NSString *reason = [NSString stringWithFormat:NSLocalizedString(@"Server HTTP error %ld.\n\n%@", nil),
status, [self extractReadableHTML:data]];
error = [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorBadServerResponse userInfo:@{NSLocalizedDescriptionKey: reason}];
data = nil;
}
block(data, error, httpResponse); // if status == 304, data & error nil block(data, error, httpResponse); // if status == 304, data & error nil
}] resume]; }] resume];
} }
/// Helper method to extract readable text from HTML
+ (NSString*)extractReadableHTML:(NSData*)data {
NSString *str = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
// replace all <tags> with (presumably) non-used character
str = [[NSRegularExpression regularExpressionWithPattern:@"<[^>]*>\\s*" options:kNilOptions error:nil]
stringByReplacingMatchesInString:str options:kNilOptions range:NSMakeRange(0, str.length) withTemplate:@"◊"];
// then replace multiple occurences of that character with a single new line
str = [[NSRegularExpression regularExpressionWithPattern:@"◊+" options:kNilOptions error:nil]
stringByReplacingMatchesInString:str options:kNilOptions range:NSMakeRange(0, str.length) withTemplate:@"\n"];
// finally trim whitespace at start and end
return [str stringByTrimmingCharactersInSet: NSCharacterSet.whitespaceAndNewlineCharacterSet];
}
#pragma mark - Download RSS Feed - #pragma mark - Download RSS Feed -
@@ -266,9 +286,9 @@ static BOOL _nextUpdateIsForced = NO;
dispatch_sync(dispatch_get_main_queue(), ^{ // sync! (thread is already in background) dispatch_sync(dispatch_get_main_queue(), ^{ // sync! (thread is already in background)
chosenURL = askUser(parsedMeta); chosenURL = askUser(parsedMeta);
}); });
if (!chosenURL || chosenURL.length == 0) { if (!chosenURL || chosenURL.length == 0) { // User canceled operation, show appropriate error message
// User canceled operation, show appropriate error message NSString *reason = NSLocalizedString(@"Operation canceled.", nil);
*err = [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorCancelled userInfo:@{NSLocalizedDescriptionKey: NSLocalizedString(@"Operation canceled.", nil)}]; *err = [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorCancelled userInfo:@{NSLocalizedDescriptionKey: reason}];
return NO; return NO;
} }
[self parseFeedRequest:[self newRequestURL:chosenURL] xmlBlock:nil feedBlock:block]; [self parseFeedRequest:[self newRequestURL:chosenURL] xmlBlock:nil feedBlock:block];

View File

@@ -56,7 +56,7 @@ NS_INLINE CGFloat YFromTop(NSView *view) { return NSHeight(view.superview.frame)
// UI: Buttons // UI: Buttons
+ (NSButton*)button:(NSString*)text; + (NSButton*)button:(NSString*)text;
+ (NSButton*)buttonImageSquare:(nonnull NSImageName)name; + (NSButton*)buttonImageSquare:(nonnull NSImageName)name;
+ (NSButton*)buttonIcon:(NSImage*)img size:(CGFloat)size; + (NSButton*)buttonIcon:(nonnull NSImageName)name size:(CGFloat)size;
+ (NSButton*)inlineButton:(NSString*)text; + (NSButton*)inlineButton:(NSString*)text;
+ (NSPopUpButton*)popupButton:(CGFloat)w; + (NSPopUpButton*)popupButton:(CGFloat)w;
// UI: Others // UI: Others

View File

@@ -87,11 +87,11 @@
} }
/// Create pure image button with no border. /// Create pure image button with no border.
+ (NSButton*)buttonIcon:(NSImage*)img size:(CGFloat)size { + (NSButton*)buttonIcon:(nonnull NSImageName)name size:(CGFloat)size {
NSButton *btn = [[NSButton alloc] initWithFrame: NSMakeRect(0, 0, size, size)]; NSButton *btn = [[NSButton alloc] initWithFrame: NSMakeRect(0, 0, size, size)];
btn.bezelStyle = NSBezelStyleRounded; btn.bezelStyle = NSBezelStyleRounded;
btn.bordered = NO; btn.bordered = NO;
btn.image = img; btn.image = [NSImage imageNamed:name];
return btn; return btn;
} }
@@ -271,6 +271,8 @@
self.toolTip = tt; self.toolTip = tt;
if (self.accessibilityLabel.length == 0) if (self.accessibilityLabel.length == 0)
self.accessibilityLabel = tt; self.accessibilityLabel = tt;
else
self.accessibilityValueDescription = tt;
return self; return self;
} }

View File

@@ -32,7 +32,7 @@
</dict> </dict>
</array> </array>
<key>CFBundleVersion</key> <key>CFBundleVersion</key>
<string>7539</string> <string>7702</string>
<key>LSMinimumSystemVersion</key> <key>LSMinimumSystemVersion</key>
<string>$(MACOSX_DEPLOYMENT_TARGET)</string> <string>$(MACOSX_DEPLOYMENT_TARGET)</string>
<key>LSUIElement</key> <key>LSUIElement</key>

View File

@@ -146,6 +146,7 @@
self.httpEtag = nil; self.httpEtag = nil;
self.httpDate = nil; self.httpDate = nil;
self.faviconURL = nil; self.faviconURL = nil;
self.previousURL = self.view.url.stringValue;
} }
/** /**
@@ -179,8 +180,11 @@
@return Either URL string or @c nil if user canceled the selection. @return Either URL string or @c nil if user canceled the selection.
*/ */
- (NSString*)letUserChooseXmlUrlFromList:(NSArray<RSHTMLMetadataFeedLink*> *)list { - (NSString*)letUserChooseXmlUrlFromList:(NSArray<RSHTMLMetadataFeedLink*> *)list {
if (list.count == 1) // nothing to choose if (list.count == 1) { // nothing to choose
// Feeds like https://news.ycombinator.com/ return 503 if URLs are requested too rapidly
//CFRunLoopRunInMode(kCFRunLoopDefaultMode, 1.0, false); // Non-blocking sleep (1s)
return list.firstObject.link; return list.firstObject.link;
}
NSMenu *menu = [[NSMenu alloc] initWithTitle:NSLocalizedString(@"Choose feed menu", nil)]; NSMenu *menu = [[NSMenu alloc] initWithTitle:NSLocalizedString(@"Choose feed menu", nil)];
menu.autoenablesItems = NO; menu.autoenablesItems = NO;
for (RSHTMLMetadataFeedLink *fl in list) { for (RSHTMLMetadataFeedLink *fl in list) {
@@ -203,11 +207,19 @@
- (void)postDownload:(NSString*)responseURL { - (void)postDownload:(NSString*)responseURL {
if (self.modalSheet.didCloseAndCancel) if (self.modalSheet.didCloseAndCancel)
return; return;
BOOL hasError = (self.feedError != nil);
// 1. Stop spinner animation for name field. (keep spinner for URL running until favicon downloaded) // 1. Stop spinner animation for name field. (keep spinner for URL running until favicon downloaded)
[self.view.spinnerName stopAnimation:nil]; [self.view.spinnerName stopAnimation:nil];
// 2. If URL was redirected, replace original text field value with new one. (e.g., https redirect) // 2. If URL was redirected, replace original text field value with new one. (e.g., https redirect)
if (responseURL.length > 0 && ![responseURL isEqualToString:self.previousURL]) { if (responseURL.length > 0 && ![responseURL isEqualToString:self.previousURL]) {
if (!hasError) {
// If the url has changed and there is an error:
// This probably means the feed URL was resolved, but the successive download returned 5xx error.
// Presumably to prevent site crawlers accessing many pages in quick succession. (delay of 1s does help)
// By not setting previousURL, a second hit on the 'Done' button will retry the resolved URL again.
self.previousURL = responseURL; self.previousURL = responseURL;
}
self.view.url.stringValue = responseURL; self.view.url.stringValue = responseURL;
} }
// 3. Copy parsed feed title to text field. (only if user hasn't set anything else yet) // 3. Copy parsed feed title to text field. (only if user hasn't set anything else yet)
@@ -218,7 +230,6 @@
// TODO: user preference to automatically select refresh interval (selection: None,min,max,avg,median) // TODO: user preference to automatically select refresh interval (selection: None,min,max,avg,median)
[self statsForDownloadObject]; [self statsForDownloadObject];
// 4. Continue with favicon download (or finish with error) // 4. Continue with favicon download (or finish with error)
BOOL hasError = (self.feedError != nil);
self.view.favicon.hidden = hasError; self.view.favicon.hidden = hasError;
self.view.warningButton.hidden = !hasError; self.view.warningButton.hidden = !hasError;
if (hasError) { if (hasError) {
@@ -305,7 +316,6 @@
- (void)controlTextDidEndEditing:(NSNotification *)obj { - (void)controlTextDidEndEditing:(NSNotification *)obj {
if (obj.object == self.view.url) { if (obj.object == self.view.url) {
if (![self.previousURL isEqualToString:self.view.url.stringValue]) { if (![self.previousURL isEqualToString:self.view.url.stringValue]) {
self.previousURL = self.view.url.stringValue;
[self downloadRSS]; [self downloadRSS];
} }
} }
@@ -316,15 +326,26 @@
if (!self.feedError) if (!self.feedError)
return; return;
// show reload button if server is temporarily offline (any 5xx server error)
BOOL serverError = (self.feedError.domain == NSURLErrorDomain && self.feedError.code == NSURLErrorBadServerResponse);
self.view.warningReload.hidden = !serverError;
// set error description as text
self.view.warningText.objectValue = self.feedError.localizedDescription; self.view.warningText.objectValue = self.feedError.localizedDescription;
NSSize newSize = self.view.warningText.fittingSize; // width is limited by the textfield's preferred width NSSize newSize = self.view.warningText.fittingSize; // width is limited by the textfield's preferred width
newSize.width += 2 * self.view.warningText.frame.origin.x; // the padding newSize.width += 2 * self.view.warningText.frame.origin.x; // the padding
newSize.height += 2 * self.view.warningText.frame.origin.y; newSize.height += 2 * self.view.warningText.frame.origin.y;
// apply fitting size and display
self.view.warningPopover.contentSize = newSize; self.view.warningPopover.contentSize = newSize;
[self.view.warningPopover showRelativeToRect:sender.bounds ofView:sender preferredEdge:NSRectEdgeMinY]; [self.view.warningPopover showRelativeToRect:sender.bounds ofView:sender preferredEdge:NSRectEdgeMinY];
} }
/// Either hit by Cmd+R or reload button inside warning popover error description
- (void)reloadData {
[self downloadRSS];
}
@end @end

View File

@@ -38,6 +38,7 @@
@property (weak) IBOutlet NSButton *warningButton; @property (weak) IBOutlet NSButton *warningButton;
@property NSPopover *warningPopover; @property NSPopover *warningPopover;
@property (weak) IBOutlet NSTextField *warningText; @property (weak) IBOutlet NSTextField *warningText;
@property (weak) IBOutlet NSButton *warningReload;
- (instancetype)initWithController:(ModalFeedEdit*)controller NS_DESIGNATED_INITIALIZER; - (instancetype)initWithController:(ModalFeedEdit*)controller NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithFrame:(NSRect)frameRect NS_UNAVAILABLE; - (instancetype)initWithFrame:(NSRect)frameRect NS_UNAVAILABLE;

View File

@@ -47,10 +47,7 @@
self.url = [[[NSView inputField:@"https://example.org/feed.rss" width:0] placeIn:self x:x yTop:0] sizeToRight:PAD_S + 18]; self.url = [[[NSView inputField:@"https://example.org/feed.rss" width:0] placeIn:self x:x yTop:0] sizeToRight:PAD_S + 18];
self.spinnerURL = [[NSView activitySpinner] placeIn:self xRight:1 yTop:2.5]; self.spinnerURL = [[NSView activitySpinner] placeIn:self xRight:1 yTop:2.5];
self.favicon = [[[NSView imageView:nil size:18] tooltip:NSLocalizedString(@"Favicon", nil)] placeIn:self xRight:0 yTop:1.5]; self.favicon = [[[NSView imageView:nil size:18] tooltip:NSLocalizedString(@"Favicon", nil)] placeIn:self xRight:0 yTop:1.5];
NSTextField *errorDesc = [self warningPopoverContentView]; self.warningButton = [[[[NSView buttonIcon:NSImageNameCaution size:18] action:@selector(didClickWarningButton:) target:nil] // up the responder chain
self.warningPopover = [self warningPopoverControllerWith:errorDesc];
self.warningText = errorDesc; // after added to parent view, otherwise will be released immediatelly (weak ivar)
self.warningButton = [[[[NSView buttonIcon:[NSImage imageNamed:NSImageNameCaution] size:18] action:@selector(didClickWarningButton:) target:nil] // up the responder chain
tooltip:NSLocalizedString(@"Click here to show failure reason", nil)] tooltip:NSLocalizedString(@"Click here to show failure reason", nil)]
placeIn:self xRight:0 yTop:1.5]; placeIn:self xRight:0 yTop:1.5];
// 2. row // 2. row
@@ -61,34 +58,39 @@
self.refreshUnit = [[NSView popupButton:120] placeIn:self x:NSMaxX(self.refreshNum.frame) + PAD_M yTop:2*rowHeight]; self.refreshUnit = [[NSView popupButton:120] placeIn:self x:NSMaxX(self.refreshNum.frame) + PAD_M yTop:2*rowHeight];
// initial state // initial state
self.url.accessibilityLabel = lbls[0];
self.name.accessibilityLabel = lbls[1];
self.refreshNum.accessibilityLabel = NSLocalizedString(@"Refresh interval", nil);
self.url.delegate = controller; self.url.delegate = controller;
self.warningButton.hidden = YES; self.warningButton.hidden = YES;
self.refreshNum.formatter = [StrictUIntFormatter new]; // see below ... self.refreshNum.formatter = [StrictUIntFormatter new]; // see below ...
//[self.warningButton.cell setHighlightsBy:(error ? NSContentsCellMask : NSNoCellMask)]; [self prepareWarningPopover];
return self; return self;
} }
/// User visible error description text (after click on warning button) /// Prepare popover controller to display errors during download
- (NSTextField*)warningPopoverContentView { - (void)prepareWarningPopover {
NSTextField *txt = [[[NSView label:@""] selectable] sizableWidthAndHeight];
[txt setFrameSize: NSMakeSize(300, 100)];
txt.lineBreakMode = NSLineBreakByWordWrapping;
txt.maximumNumberOfLines = 7;
return txt;
}
/// Prepare popover controller to display download errors
- (NSPopover*)warningPopoverControllerWith:(NSTextField*)content {
NSPopover *pop = [[NSPopover alloc] init]; NSPopover *pop = [[NSPopover alloc] init];
pop.behavior = NSPopoverBehaviorTransient; pop.behavior = NSPopoverBehaviorTransient;
pop.contentViewController = [[NSViewController alloc] init]; pop.contentViewController = [[NSViewController alloc] init];
pop.contentViewController.view = [[NSView alloc] initWithFrame:content.frame]; NSView *content = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 300, 100)];
[pop.contentViewController.view addSubview:content]; pop.contentViewController.view = content;
content.frame = NSInsetRect(content.frame, 4, 2); // User visible error description text (after click on warning button)
content.preferredMaxLayoutWidth = NSWidth(content.frame); NSTextField *txt = [[[NSView label:@""] selectable] sizableWidthAndHeight];
return pop; txt.frame = NSInsetRect(content.frame, 4, 2);
txt.preferredMaxLayoutWidth = NSWidth(txt.frame);
txt.lineBreakMode = NSLineBreakByWordWrapping;
txt.maximumNumberOfLines = 7;
[content addSubview:txt];
self.warningPopover = pop;
self.warningText = txt;
// Reload button is only visible on 5xx server error (right of )
self.warningReload = [[[[NSView buttonIcon:NSImageNameRefreshTemplate size:16] placeIn:content x:35 yTop:21]
tooltip:NSLocalizedString(@"Retry download (Cmd+R)", nil)]
action:@selector(reloadData) target:nil]; // up the responder chain
} }
@end @end

View File

@@ -134,7 +134,7 @@
*/ */
- (CGFloat)generateButtons { - (CGFloat)generateButtons {
NSButton *add = [[NSView buttonImageSquare:NSImageNameAddTemplate] tooltip:NSLocalizedString(@"Add new item", nil)]; NSButton *add = [[NSView buttonImageSquare:NSImageNameAddTemplate] tooltip:NSLocalizedString(@"Add new item", nil)];
NSButton *del = [[NSView buttonImageSquare:NSImageNameRemoveTemplate] tooltip:NSLocalizedString(@"Delete selected item(s)", nil)]; NSButton *del = [[NSView buttonImageSquare:NSImageNameRemoveTemplate] tooltip:NSLocalizedString(@"Delete selected items", nil)];
NSButton *share = [[NSView buttonImageSquare:NSImageNameShareTemplate] tooltip:NSLocalizedString(@"Import or export data", nil)]; NSButton *share = [[NSView buttonImageSquare:NSImageNameShareTemplate] tooltip:NSLocalizedString(@"Import or export data", nil)];
[self button:add copyActions:3 to:5]; [self button:add copyActions:3 to:5];