From b29be4ac2b04089776584c0a8ee9b735e2c258f0 Mon Sep 17 00:00:00 2001 From: Opender Singh Date: Tue, 28 Jul 2020 11:48:55 +1200 Subject: [PATCH] Handle silent failures for git push operation (#2432) --- .eslintrc.json | 4 ++- .../app/__mocks__/isomorphic-git.js | 6 ++++ .../app/sync/git/__tests__/git-vcs.test.js | 16 ++++++++++ packages/insomnia-app/app/sync/git/git-vcs.js | 31 +++++++++++++++++-- .../editors/body/graph-ql-editor.js | 2 +- .../app/ui/components/modals/error-modal.js | 4 +-- 6 files changed, 57 insertions(+), 6 deletions(-) create mode 100644 packages/insomnia-app/app/__mocks__/isomorphic-git.js diff --git a/.eslintrc.json b/.eslintrc.json index 87756154c..0674b6bc8 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -40,7 +40,9 @@ "asyncArrow": "always" } ], - "filenames/match-exported": ["error", "kebab"] + "filenames/match-exported": ["error", "kebab"], + "flowtype/array-style-simple-type": "error", + "flowtype/array-style-complex-type": "error" }, "settings": { "flowtype": { diff --git a/packages/insomnia-app/app/__mocks__/isomorphic-git.js b/packages/insomnia-app/app/__mocks__/isomorphic-git.js new file mode 100644 index 000000000..9bac57ecd --- /dev/null +++ b/packages/insomnia-app/app/__mocks__/isomorphic-git.js @@ -0,0 +1,6 @@ +// eslint-disable-next-line filenames/match-exported +const git = jest.requireActual('isomorphic-git'); +const mock = jest.genMockFromModule('isomorphic-git'); + +git.push = mock.push; +module.exports = git; diff --git a/packages/insomnia-app/app/sync/git/__tests__/git-vcs.test.js b/packages/insomnia-app/app/sync/git/__tests__/git-vcs.test.js index fd387bb1b..c0315d933 100644 --- a/packages/insomnia-app/app/sync/git/__tests__/git-vcs.test.js +++ b/packages/insomnia-app/app/sync/git/__tests__/git-vcs.test.js @@ -2,6 +2,8 @@ import GitVCS, { GIT_CLONE_DIR, GIT_INSOMNIA_DIR } from '../git-vcs'; import { setupDateMocks } from './util'; import { MemPlugin } from '../mem-plugin'; import path from 'path'; +import * as git from 'isomorphic-git'; + jest.mock('path'); describe.each(['win32', 'posix'])('Git-VCS using path.%s', type => { @@ -144,6 +146,20 @@ describe.each(['win32', 'posix'])('Git-VCS using path.%s', type => { }); }); + describe('push()', () => { + it('should throw an exception when push response contains errors', async () => { + git.push.mockReturnValue({ + ok: ['unpack'], + errors: ['refs/heads/master pre-receive hook declined'], + }); + + const vcs = new GitVCS(); + await expect(vcs.push()).rejects.toThrowError( + 'Push rejected with errors: ["refs/heads/master pre-receive hook declined"].\n\nGo to View > Toggle DevTools > Console for more information.', + ); + }); + }); + describe('undoPendingChanges()', () => { it('should remove pending changes from all tracked files', async () => { const folder = path.join(GIT_INSOMNIA_DIR, 'folder'); diff --git a/packages/insomnia-app/app/sync/git/git-vcs.js b/packages/insomnia-app/app/sync/git/git-vcs.js index adfc625c0..f98a94ea2 100644 --- a/packages/insomnia-app/app/sync/git/git-vcs.js +++ b/packages/insomnia-app/app/sync/git/git-vcs.js @@ -4,6 +4,7 @@ import { trackEvent } from '../../common/analytics'; import { httpPlugin } from './http'; import { convertToOsSep, convertToPosixSep } from './path-sep'; import path from 'path'; +import EventEmitter from 'events'; export type GitAuthor = {| name: string, @@ -36,6 +37,12 @@ export type GitLogEntry = {| }, |}; +export type PushResponse = { + ok?: Array, + errors?: Array, + headers?: object, +}; + // isomorphic-git internally will default an empty ('') clone directory to '.' // Ref: https://github.com/isomorphic-git/isomorphic-git/blob/4e66704d05042624bbc78b85ee5110d5ee7ec3e2/src/utils/normalizePath.js#L10 // We should set this explicitly (even if set to an empty string), because we have other code (such as fs plugins @@ -60,6 +67,12 @@ export default class GitVCS { this._git = git; git.plugins.set('fs', fsPlugin); git.plugins.set('http', httpPlugin); + const emitter = new EventEmitter(); + git.plugins.set('emitter', emitter); + + emitter.on('message', message => { + console.log(`[git-event] ${message}`); + }); this._baseOpts = { dir: directory, gitdir: gitDirectory }; @@ -229,11 +242,25 @@ export default class GitVCS { return true; } - async push(creds?: GitCredentials | null, force: boolean = false): Promise { + async push(creds?: GitCredentials | null, force: boolean = false): Promise { console.log(`[git] Push remote=origin force=${force ? 'true' : 'false'}`); trackEvent('Git', 'Push'); - return git.push({ ...this._baseOpts, remote: 'origin', ...creds, force }); + // eslint-disable-next-line no-unreachable + const response: PushResponse = await git.push({ + ...this._baseOpts, + remote: 'origin', + ...creds, + force, + }); + + if (response.errors?.length) { + console.log(`[git] Push rejected`, response); + const errorsString = JSON.stringify(response.errors); + throw new Error( + `Push rejected with errors: ${errorsString}.\n\nGo to View > Toggle DevTools > Console for more information.`, + ); + } } async pull(creds?: GitCredentials | null): Promise { diff --git a/packages/insomnia-app/app/ui/components/editors/body/graph-ql-editor.js b/packages/insomnia-app/app/ui/components/editors/body/graph-ql-editor.js index 4562369c5..49d06f55a 100644 --- a/packages/insomnia-app/app/ui/components/editors/body/graph-ql-editor.js +++ b/packages/insomnia-app/app/ui/components/editors/body/graph-ql-editor.js @@ -77,7 +77,7 @@ type State = { @autobind class GraphQLEditor extends React.PureComponent { - _disabledOperationMarkers: TextMarker[]; + _disabledOperationMarkers: Array; _documentAST: null | Object; _isMounted: boolean; _queryEditor: null | CodeMirror; diff --git a/packages/insomnia-app/app/ui/components/modals/error-modal.js b/packages/insomnia-app/app/ui/components/modals/error-modal.js index 9f2ddf3b0..b891d9a55 100644 --- a/packages/insomnia-app/app/ui/components/modals/error-modal.js +++ b/packages/insomnia-app/app/ui/components/modals/error-modal.js @@ -62,12 +62,12 @@ class ErrorModal extends PureComponent<{}, ErrorModalOptions> { {title || 'Uh Oh!'} - {message ?
{message}
: null} + {message ?
{message}
: null} {error && (
Stack trace
-                {error.stack || error}
+                {error.stack || error}
               
)}