[git] Use common error handler

* Fixes error notifications for `GitQuickOpenService`
* Fixes potential NPEs if error is undefined

Signed-off-by: Alex Tugarev <alex.tugarev@typefox.io>

packages/git/src/browser/git-error-handler.ts

@@ -0,0 +1,33 @@
+/********************************************************************************
+ * Copyright (C) 2018 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { injectable, inject } from "inversify";
+import { MessageService } from "@theia/core";
+
+@injectable()
+export class GitErrorHandler {
+
+    @inject(MessageService) protected readonly messageService: MessageService;
+
+    // tslint:disable-next-line:no-any
+    public handleError(error: any): void {
+        const message = error instanceof Error ? error.message : error;
+        if (message) {
+            this.messageService.error(message, { timeout: 0 });
+        }
+    }
+
+}

packages/git/src/browser/git-frontend-module.ts

@@ -34,6 +34,7 @@ import { bindBlame } from './blame/blame-module';
 import { GitRepositoryTracker } from './git-repository-tracker';
 import { GitCommitMessageValidator } from './git-commit-message-validator';
 import { GitSyncService } from './git-sync-service';
+import { GitErrorHandler } from './git-error-handler';
 
 import '../../src/browser/style/index.css';
 
@@ -70,4 +71,5 @@ export default new ContainerModule(bind => {
     bind(GitCommitMessageValidator).toSelf().inSingletonScope();
 
     bind(GitSyncService).toSelf().inSingletonScope();
+    bind(GitErrorHandler).toSelf().inSingletonScope();
 });

packages/git/src/browser/git-quick-open-service.ts

@@ -22,6 +22,7 @@ import { GitRepositoryProvider } from './git-repository-provider';
 import { MessageService } from '@theia/core/lib/common/message-service';
 import URI from '@theia/core/lib/common/uri';
 import { FileUri } from '@theia/core/lib/node/file-uri';
+import { GitErrorHandler } from "./git-error-handler";
 
 /**
  * Service delegating into the `Quick Open Service`, so that the Git commands can be further refined.
@@ -31,6 +32,8 @@ import { FileUri } from '@theia/core/lib/node/file-uri';
 @injectable()
 export class GitQuickOpenService {
 
+    @inject(GitErrorHandler) protected readonly gitErrorHandler: GitErrorHandler;
+
     constructor(
         @inject(Git) protected readonly git: Git,
         @inject(GitRepositoryProvider) protected readonly repositoryProvider: GitRepositoryProvider,
@@ -46,7 +49,7 @@ export class GitQuickOpenService {
                 try {
                     await this.git.fetch(repository, { remote: item.getLabel() });
                 } catch (error) {
-                    this.logError(error);
+                    this.gitErrorHandler.handleError(error);
                 }
             };
             const items = remotes.map(remote => new GitQuickOpenItem(remote, execute));
@@ -62,7 +65,7 @@ export class GitQuickOpenService {
                 try {
                     await this.git.push(repository, { remote: item.getLabel() });
                 } catch (error) {
-                    this.logError(error);
+                    this.gitErrorHandler.handleError(error);
                 }
             };
             const items = remotes.map(remote => new GitQuickOpenItem(remote, execute));
@@ -82,7 +85,7 @@ export class GitQuickOpenService {
                     try {
                         await this.git.pull(repository, { remote: remoteItem.getLabel() });
                     } catch (error) {
-                        this.logError(error);
+                        this.gitErrorHandler.handleError(error);
                     }
                 } else {
                     // Otherwise we need to propose the branches from
@@ -91,7 +94,7 @@ export class GitQuickOpenService {
                         try {
                             await this.git.pull(repository, { remote: remoteItem.ref, branch: branchItem.ref.nameWithoutRemote });
                         } catch (error) {
-                            this.logError(error);
+                            this.gitErrorHandler.handleError(error);
                         }
                     };
                     const toLabel = (branchItem: GitQuickOpenItem<Branch>) => branchItem.ref.name;
@@ -115,7 +118,7 @@ export class GitQuickOpenService {
                 try {
                     await this.git.merge(repository, { branch: item.getLabel()! });
                 } catch (error) {
-                    this.logError(error);
+                    this.gitErrorHandler.handleError(error);
                 }
             };
             const toLabel = (item: GitQuickOpenItem<Branch>) => item.ref.name;
@@ -138,7 +141,7 @@ export class GitQuickOpenService {
                 try {
                     await this.git.checkout(repository, { branch: item.ref.nameWithoutRemote });
                 } catch (error) {
-                    this.logError(error);
+                    this.gitErrorHandler.handleError(error);
                 }
             };
             const toLabel = (item: GitQuickOpenItem<Branch>) => {
@@ -168,7 +171,7 @@ export class GitQuickOpenService {
                                         await __this.git.branch(repository, { toCreate: lookFor });
                                         await __this.git.checkout(repository, { branch: lookFor });
                                     } catch (error) {
-                                        __this.logError(error);
+                                        __this.gitErrorHandler.handleError(error);
                                     }
                                 }
                             ));
@@ -275,7 +278,7 @@ export class GitQuickOpenService {
         try {
             return repository ? await this.git.remote(repository) : [];
         } catch (error) {
-            this.logError(error);
+            this.gitErrorHandler.handleError(error);
             return [];
         }
     }
@@ -301,7 +304,7 @@ export class GitQuickOpenService {
             ]);
             return [...local, ...remote];
         } catch (error) {
-            this.logError(error);
+            this.gitErrorHandler.handleError(error);
             return [];
         }
     }
@@ -314,17 +317,11 @@ export class GitQuickOpenService {
         try {
             return await this.git.branch(repository, { type: 'current' });
         } catch (error) {
-            this.logError(error);
+            this.gitErrorHandler.handleError(error);
             return undefined;
         }
     }
 
-    // tslint:disable-next-line:no-any
-    private logError(error: any): void {
-        const message = error instanceof Error ? error.message : error;
-        this.messageService.error(message);
-    }
-
 }
 
 /**

packages/git/src/browser/git-sync-service.ts

@@ -19,6 +19,7 @@ import { MessageService, Emitter, Event } from "@theia/core";
 import { QuickOpenService, QuickOpenItem, QuickOpenMode, ConfirmDialog } from "@theia/core/lib/browser";
 import { GitRepositoryTracker } from "./git-repository-tracker";
 import { Git, Repository, WorkingDirectoryStatus } from "../common";
+import { GitErrorHandler } from "./git-error-handler";
 
 @injectable()
 export class GitSyncService {
@@ -32,6 +33,9 @@ export class GitSyncService {
     @inject(MessageService)
     protected readonly messageService: MessageService;
 
+    @inject(GitErrorHandler)
+    protected readonly gitErrorHandler: GitErrorHandler;
+
     @inject(QuickOpenService)
     protected readonly quickOpenService: QuickOpenService;
 
@@ -85,8 +89,8 @@ export class GitSyncService {
                     force: method === 'force-push'
                 });
             }
-        } catch (e) {
-            this.error(e);
+        } catch (error) {
+            this.gitErrorHandler.handleError(error);
         } finally {
             this.setSyncing(false);
         }
@@ -146,8 +150,8 @@ export class GitSyncService {
                 await this.git.push(repository, {
                     remote, localBranch, setUpstream: true
                 });
-            } catch (e) {
-                this.error(e);
+            } catch (error) {
+                this.gitErrorHandler.handleError(error);
             }
         }
     }
@@ -200,18 +204,6 @@ export class GitSyncService {
         return new ConfirmDialog({ title, msg, }).open();
     }
 
-    // tslint:disable-next-line:no-any
-    protected error(e: any): void {
-        if ('message' in e) {
-            const message = e['message'];
-            if (typeof message === "string" && message.startsWith('GitError')) {
-                this.messageService.error(message, { timeout: 0 });
-                return;
-            }
-        }
-        throw e;
-    }
-
 }
 export namespace GitSyncService {
     export type SyncMethod = 'pull-push' | 'rebase-push' | 'force-push';

packages/git/src/browser/git-widget.tsx

@@ -16,7 +16,7 @@
 
 import { injectable, inject, postConstruct } from 'inversify';
 import URI from '@theia/core/lib/common/uri';
-import { MessageService, ResourceProvider, CommandService, MenuPath } from '@theia/core';
+import { ResourceProvider, CommandService, MenuPath } from '@theia/core';
 import { ContextMenuRenderer, LabelProvider, DiffUris, StatefulWidget, Message } from '@theia/core/lib/browser';
 import { EditorManager, EditorWidget, EditorOpenerOptions } from '@theia/editor/lib/browser';
 import { WorkspaceService, WorkspaceCommands } from '@theia/workspace/lib/browser';
@@ -28,6 +28,7 @@ import { GitCommitMessageValidator } from './git-commit-message-validator';
 import { GitAvatarService } from './history/git-avatar-service';
 import { ReactWidget } from '@theia/core/lib/browser/widgets/react-widget';
 import * as React from "react";
+import { GitErrorHandler } from './git-error-handler';
 
 export interface GitFileChangeNode extends GitFileChange {
     readonly icon: string;
@@ -63,12 +64,14 @@ export class GitWidget extends ReactWidget implements StatefulWidget {
     @inject(EditorManager)
     protected readonly editorManager: EditorManager;
 
+    @inject(GitErrorHandler)
+    protected readonly gitErrorHandler: GitErrorHandler;
+
     constructor(
         @inject(Git) protected readonly git: Git,
         @inject(GitWatcher) protected readonly gitWatcher: GitWatcher,
         @inject(ContextMenuRenderer) protected readonly contextMenuRenderer: ContextMenuRenderer,
         @inject(ResourceProvider) protected readonly resourceProvider: ResourceProvider,
-        @inject(MessageService) protected readonly messageService: MessageService,
         @inject(CommandService) protected readonly commandService: CommandService,
         @inject(GitRepositoryProvider) protected readonly repositoryProvider: GitRepositoryProvider,
         @inject(LabelProvider) protected readonly labelProvider: LabelProvider,
@@ -173,7 +176,7 @@ export class GitWidget extends ReactWidget implements StatefulWidget {
                     this.resetCommitMessages();
                     this.updateView(status);
                 } catch (error) {
-                    this.logError(error);
+                    this.gitErrorHandler.handleError(error);
                 }
             } else {
                 const messageInput = document.getElementById(GitWidget.Styles.COMMIT_MESSAGE) as HTMLInputElement;
@@ -430,7 +433,7 @@ export class GitWidget extends ReactWidget implements StatefulWidget {
         try {
             await this.git.unstage(repository, change.uri);
         } catch (error) {
-            this.logError(error);
+            this.gitErrorHandler.handleError(error);
         }
     }
 
@@ -443,7 +446,7 @@ export class GitWidget extends ReactWidget implements StatefulWidget {
             try {
                 await this.git.unstage(repository, change.uri, { treeish: 'HEAD', reset: 'working-tree' });
             } catch (error) {
-                this.logError(error);
+                this.gitErrorHandler.handleError(error);
             }
         } else {
             this.commandService.executeCommand(WorkspaceCommands.FILE_DELETE.id, new URI(change.uri));
@@ -457,7 +460,7 @@ export class GitWidget extends ReactWidget implements StatefulWidget {
         try {
             await this.git.add(repository, change.uri);
         } catch (error) {
-            this.logError(error);
+            this.gitErrorHandler.handleError(error);
         }
     }
 
@@ -550,12 +553,6 @@ export class GitWidget extends ReactWidget implements StatefulWidget {
         return undefined;
     }
 
-    // tslint:disable-next-line:no-any
-    protected logError(error: any): void {
-        const message = error instanceof Error ? error.message : error;
-        this.messageService.error(message, { timeout: 0 });
-    }
-
     findChange(uri: URI): GitFileChange | undefined {
         const stringUri = uri.toString();
         const merge = this.mergeChanges.find(c => c.uri.toString() === stringUri);