Skip to content

Commit

Permalink
Improve error message for audio-only files
Browse files Browse the repository at this point in the history
  • Loading branch information
sindresorhus committed Apr 11, 2019
1 parent c6cd8db commit f91e377
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 59 deletions.
5 changes: 2 additions & 3 deletions Gifski/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
guard urls.count == 1 else {
NSAlert.showModal(
for: mainWindowController.window,
title: "Max one file",
message: "You can only convert a single file at the time"
message: "Gifski can only convert a single file at the time."
)
return
}
Expand All @@ -61,7 +60,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
}

func application(_ application: NSApplication, willPresentError error: Error) -> Error {
Crashlytics.sharedInstance().recordErrorBetter(error)
Crashlytics.recordNonFatalError(error: error)
return error
}
}
11 changes: 7 additions & 4 deletions Gifski/Base.lproj/MainMenu.xib
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="14113" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" customObjectInstantitationMethod="direct">
<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="14490.70" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none" customObjectInstantitationMethod="direct">
<dependencies>
<plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="14113"/>
<deployment identifier="macosx"/>
<plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="14490.70"/>
</dependencies>
<objects>
<customObject id="-2" userLabel="File's Owner" customClass="NSApplication">
Expand Down Expand Up @@ -141,24 +142,26 @@
<modifierMask key="keyEquivalentModifierMask"/>
<menu key="submenu" title="Help" systemMenu="help" id="F2S-fz-NVQ">
<items>
<menuItem title="Gifski.app Website" id="IJ2-L8-CcF" customClass="UrlMenuItem" customModule="Gifski" customModuleProvider="target">
<menuItem title="Website" id="IJ2-L8-CcF" customClass="UrlMenuItem" customModule="Gifski" customModuleProvider="target">
<modifierMask key="keyEquivalentModifierMask"/>
<userDefinedRuntimeAttributes>
<userDefinedRuntimeAttribute type="string" keyPath="url" value="https://sindresorhus.com/gifski"/>
</userDefinedRuntimeAttributes>
</menuItem>
<menuItem title="Gifski.app Source Code" id="LPR-FD-PrP" customClass="UrlMenuItem" customModule="Gifski" customModuleProvider="target">
<menuItem title="Source Code" id="LPR-FD-PrP" customClass="UrlMenuItem" customModule="Gifski" customModuleProvider="target">
<modifierMask key="keyEquivalentModifierMask"/>
<userDefinedRuntimeAttributes>
<userDefinedRuntimeAttribute type="string" keyPath="url" value="https://github.com/sindresorhus/gifski-app"/>
</userDefinedRuntimeAttributes>
</menuItem>
<menuItem isSeparatorItem="YES" id="R9f-81-F3Z"/>
<menuItem title="About Gifski Library" id="TcL-ZY-7a3" customClass="UrlMenuItem" customModule="Gifski" customModuleProvider="target">
<modifierMask key="keyEquivalentModifierMask"/>
<userDefinedRuntimeAttributes>
<userDefinedRuntimeAttribute type="string" keyPath="url" value="https://gif.ski"/>
</userDefinedRuntimeAttributes>
</menuItem>
<menuItem isSeparatorItem="YES" id="yQX-Pq-3me"/>
<menuItem title="Send Feedback…" id="sOe-gQ-Htm" customClass="FeedbackMenuItem" customModule="Gifski" customModuleProvider="target">
<modifierMask key="keyEquivalentModifierMask"/>
</menuItem>
Expand Down
19 changes: 19 additions & 0 deletions Gifski/Gifski.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Foundation
import AVFoundation
import Crashlytics

final class Gifski {
enum Error: LocalizedError {
Expand Down Expand Up @@ -87,6 +88,24 @@ final class Gifski {

DispatchQueue.global(qos: .utility).async {
let asset = AVURLAsset(url: conversion.input, options: nil)

Crashlytics.record(
key: "Does input file exist",
value: conversion.input.exists
)
Crashlytics.record(
key: "Is input file reachable",
value: try? conversion.input.checkResourceIsReachable()
)
Crashlytics.record(
key: "Is input file readable",
value: conversion.input.isReadable
)
Crashlytics.record(
key: "AVAsset debug info 2",
value: asset.debugInfo
)

let generator = AVAssetImageGenerator(asset: asset)
generator.requestedTimeToleranceAfter = .zero
generator.requestedTimeToleranceBefore = .zero
Expand Down
38 changes: 30 additions & 8 deletions Gifski/MainWindowController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,27 @@ final class MainWindowController: NSWindowController {
func convert(_ inputUrl: URL) {
let asset = AVURLAsset(url: inputUrl)

Crashlytics.record(key: "AVAsset debug info", value: asset.debugInfo)

guard asset.videoCodec != "rle" else {
NSAlert.showModal(
for: window,
title: "QuickTime Animation format not supported",
message: "Re-export or convert your video to ProRes 4444 XQ instead. It's more efficient, more widely supported, and like QuickTime Animation, it also supports alpha channel. To convert an existing video, just open it in QuickTime Player (which will convert it) and then save it."
message: "The QuickTime Animation format is not supported.",
informativeText: "Re-export or convert your video to ProRes 4444 XQ instead. It's more efficient, more widely supported, and like QuickTime Animation, it also supports alpha channel. To convert an existing video, open it in QuickTime Player, which will automatically convert it, and then save it."
)
return
}

if asset.hasAudio && !asset.hasVideo {
NSAlert.showModal(
for: window,
message: "Audio files are not supported.",
informativeText: "Gifski converts video files but the provided file is audio-only. Please provide a file that contains video."
)

Crashlytics.recordNonFatalError(
title: "Audio files are not supported.",
message: asset.debugInfo
)
return
}
Expand All @@ -118,22 +134,28 @@ final class MainWindowController: NSWindowController {
guard asset.isVideoDecodable else {
NSAlert.showModal(
for: window,
title: "Video file not supported",
message: "The video file you tried to convert could not be read. Please open an issue on https://github.com/sindresorhus/gifski-app. ZIP the video and attach it to the issue.\n\nInclude this info:\n\(asset.debugInfo)"
message: "The video file is not supported.",
informativeText: "Please open an issue on https://github.com/sindresorhus/gifski-app. ZIP the video and attach it to the issue.\n\nInclude this info:\n\(asset.debugInfo)"
)

Crashlytics.sharedInstance().recordErrorMessage("Video file not supported: \(asset.debugInfo)")
Crashlytics.recordNonFatalError(
title: "The video file is not supported.",
message: asset.debugInfo
)
return
}

guard let videoMetadata = asset.videoMetadata else {
NSAlert.showModal(
for: window,
title: "Video metadata not readable",
message: "The metadata of the video could not be read. Please open an issue on https://github.com/sindresorhus/gifski-app. ZIP the video and attach it to the issue.\n\nInclude this info:\n\(asset.debugInfo)"
message: "The video metadata is not readable.",
informativeText: "Please open an issue on https://github.com/sindresorhus/gifski-app. ZIP the video and attach it to the issue.\n\nInclude this info:\n\(asset.debugInfo)"
)

Crashlytics.sharedInstance().recordErrorMessage("Video metadata not readable: \(asset.debugInfo)")
Crashlytics.recordNonFatalError(
title: "The video metadata is not readable.",
message: asset.debugInfo
)
return
}

Expand Down
137 changes: 93 additions & 44 deletions Gifski/util.swift
Original file line number Diff line number Diff line change
Expand Up @@ -237,59 +237,59 @@ extension NSWindowController: NSWindowDelegate {


extension NSAlert {
/// Show a modal alert sheet on a window
/// If the window is nil, it will be a app-modal alert
/// Show a modal alert sheet on a window.
/// If the window is nil, it will be a app-modal alert.
@discardableResult
static func showModal(
for window: NSWindow?,
title: String,
message: String? = nil,
style: NSAlert.Style = .critical
message: String,
informativeText: String? = nil,
style: NSAlert.Style = .warning
) -> NSApplication.ModalResponse {
guard let window = window else {
return NSAlert(
title: title,
message: message,
informativeText: informativeText,
style: style
).runModal()
}

return NSAlert(
title: title,
message: message,
informativeText: informativeText,
style: style
).runModal(for: window)
}

/// Show a app-modal (window indepedendent) alert
/// Show a app-modal (window indepedendent) alert.
@discardableResult
static func showModal(
title: String,
message: String? = nil,
style: NSAlert.Style = .critical
message: String,
informativeText: String? = nil,
style: NSAlert.Style = .warning
) -> NSApplication.ModalResponse {
return NSAlert(
title: title,
message: message,
informativeText: informativeText,
style: style
).runModal()
}

convenience init(
title: String,
message: String? = nil,
style: NSAlert.Style = .critical
message: String,
informativeText: String? = nil,
style: NSAlert.Style = .warning
) {
self.init()
self.messageText = title
self.messageText = message
self.alertStyle = style

if let message = message {
self.informativeText = message
if let informativeText = informativeText {
self.informativeText = informativeText
}
}

/// Runs the alert as a window-modal sheel
/// Runs the alert as a window-modal sheet.
@discardableResult
func runModal(for window: NSWindow) -> NSApplication.ModalResponse {
beginSheetModal(for: window) { returnCode in
Expand Down Expand Up @@ -614,6 +614,16 @@ extension AVAsset {
return firstVideoTrack.isDecodable
}

/// Returns a boolean of whether there are any video tracks.
var hasVideo: Bool {
return !tracks(withMediaType: .video).isEmpty
}

/// Returns a boolean of whether there are any audio tracks.
var hasAudio: Bool {
return !tracks(withMediaType: .audio).isEmpty
}

/// Returns the first video track if any.
var firstVideoTrack: AVAssetTrack? {
return tracks(withMediaType: .video).first
Expand Down Expand Up @@ -1275,6 +1285,14 @@ extension URL {
return values.allValues[key] as? T
}

private func boolResourceValue(forKey key: URLResourceKey, defaultValue: Bool = false) -> Bool {
guard let values = try? resourceValues(forKeys: [key]) else {
return defaultValue
}

return values.allValues[key] as? Bool ?? defaultValue
}

/// File UTI
var typeIdentifier: String? {
return resourceValue(forKey: .typeIdentifierKey)
Expand All @@ -1288,6 +1306,14 @@ extension URL {
var fileSizeFormatted: String {
return ByteCountFormatter.string(fromByteCount: Int64(fileSize), countStyle: .file)
}

var exists: Bool {
return FileManager.default.fileExists(atPath: path)
}

var isReadable: Bool {
return boolResourceValue(forKey: .isReadableKey)
}
}

extension CGSize {
Expand Down Expand Up @@ -1693,42 +1719,64 @@ extension Error {
extension NSError {
// TODO: Return `Self` here in Swift 5.1
class func from(error: Error, userInfo: [String: Any] = [:]) -> NSError {
let nsError = error as NSError

// Since Error and NSError are often bridged between each other, we check if it was originally an NSError and then return that.
guard !error.isNsError else {
guard !userInfo.isEmpty else {
return error as NSError
return nsError
}

// TODO: Use `Self` instead of `NSError` here in Swift 5.1
return (error as NSError).appending(userInfo: userInfo)
return nsError.appending(userInfo: userInfo)
}

var userInfo = userInfo
userInfo[NSLocalizedDescriptionKey] = error.localizedDescription

// This is needed as `localizedDescription` often lacks important information, for example, when an NSError is wrapped in a Swift.Error.
userInfo["Swift.Error"] = "\(nsError.domain).\(error)"

return self.init(
domain: "\(App.id) - \((error as NSError).domain)",
code: 0,
domain: "\(App.id) - \(nsError.domain)",
code: nsError.code,
userInfo: userInfo
)
}

class func appError(message: String, userInfo: [String: Any] = [:]) -> Self {
/**
- Parameter domainPostfix: String to append to the `domain`.
*/
class func appError(message: String, userInfo: [String: Any] = [:], domainPostfix: String? = nil) -> Self {
return self.init(
domain: App.id,
domain: domainPostfix != nil ? "\(App.id) - \(domainPostfix!)" : App.id,
code: 0,
userInfo: [NSLocalizedDescriptionKey: message]
)
}

/// Returns a new error with the user info appended
func appending(userInfo: [String: Any]) -> Self {
var currentUserInfo = userInfo
for (key, value) in userInfo {
currentUserInfo[key] = value
}
/// Returns a new error with the user info appended.
func appending(userInfo newUserInfo: [String: Any]) -> Self {
// TODO: Use `Self` here in Swift 5.1
return type(of: self).init(domain: domain, code: code, userInfo: currentUserInfo)
return type(of: self).init(
domain: domain,
code: code,
userInfo: userInfo.appending(newUserInfo)
)
}
}

extension Dictionary {
/// Adds the elements of the given dictionary to a copy of self and returns that.
/// Identical keys in the given dictionary overwrites keys in the copy of self.
func appending(_ dictionary: [Key: Value]) -> [Key: Value] {
var newDictionary = self

for (key, value) in dictionary {
newDictionary[key] = value
}

return newDictionary
}
}

Expand All @@ -1737,24 +1785,25 @@ extension NSError {

extension Crashlytics {
/// A better error recording method. Captures more debug info.
func recordErrorBetter(_ error: Error) {
static func recordNonFatalError(error: Error, userInfo: [String: Any] = [:]) {
#if !DEBUG
// This forces Crashlytics to actually provide some useful info for Swift errors
let nsError = NSError.from(error: error)

recordError(
nsError,
withAdditionalUserInfo: [
"userInfo": nsError.userInfo,
"localizedDescription": nsError.localizedDescription
]
)
let nsError = NSError.from(error: error, userInfo: userInfo)

sharedInstance().recordError(nsError)
#endif
}

static func recordNonFatalError(title: String? = nil, message: String) {
#if !DEBUG
sharedInstance().recordError(NSError.appError(message: message, domainPostfix: title))
#endif
}

func recordErrorMessage(_ message: String) {
/// Set a value for a for a key to be associated with your crash data which will be visible in Crashlytics.
static func record(key: String, value: Any?) {
#if !DEBUG
recordError(NSError.appError(message: message))
sharedInstance().setObjectValue(value, forKey: key)
#endif
}
}
Expand Down

0 comments on commit f91e377

Please sign in to comment.