From 7de17f7b3fe7e218edfc16a29eb098bee3110453 Mon Sep 17 00:00:00 2001 From: abose Date: Sat, 1 Feb 2025 09:49:49 +0530 Subject: [PATCH] fix: bugsnag error logs in git when staging fails is an expected error --- .../default/Git/src/ErrorHandler.js | 28 +++++++++++----- src/extensions/default/Git/src/Panel.js | 32 +++++++++++++++---- src/nls/root/strings.js | 1 + src/styles/brackets.less | 7 ++++ src/widgets/NotificationUI.js | 9 ++++-- 5 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/extensions/default/Git/src/ErrorHandler.js b/src/extensions/default/Git/src/ErrorHandler.js index 53e6e4d402..9378171b17 100644 --- a/src/extensions/default/Git/src/ErrorHandler.js +++ b/src/extensions/default/Git/src/ErrorHandler.js @@ -4,6 +4,7 @@ define(function (require, exports) { Mustache = brackets.getModule("thirdparty/mustache/mustache"), Metrics = brackets.getModule("utils/Metrics"), Strings = brackets.getModule("strings"), + NotificationUI = brackets.getModule("widgets/NotificationUI"), Utils = require("src/Utils"), errorDialogTemplate = require("text!templates/git-error-dialog.html"); @@ -40,7 +41,7 @@ define(function (require, exports) { * * @param err * @param title - * @param {dontStripError: boolean, errorMetric: string} options + * @param {dontStripError: boolean, errorMetric: string, useNotification: boolean} options */ exports.showError = function (err, title, options = {}) { const dontStripError = options.dontStripError; @@ -67,14 +68,25 @@ define(function (require, exports) { errorBody = "Error can't be stringified by JSON.stringify"; } } + errorBody = window.debugMode ? `${errorBody}\n${errorStack}` : errorBody; + + if(options.useNotification){ + NotificationUI.createToastFromTemplate(title, + ``, { + toastStyle: NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.ERROR, + dismissOnClick: false, + instantOpen: true + }); + } else { + const compiledTemplate = Mustache.render(errorDialogTemplate, { + title: title, + body: errorBody, + Strings: Strings + }); + + Dialogs.showModalDialogUsingTemplate(compiledTemplate); + } - var compiledTemplate = Mustache.render(errorDialogTemplate, { - title: title, - body: window.debugMode ? `${errorBody}\n${errorStack}` : errorBody, - Strings: Strings - }); - - Dialogs.showModalDialogUsingTemplate(compiledTemplate); if (typeof err === "string") { err = new Error(err); } err.__shown = true; return err; diff --git a/src/extensions/default/Git/src/Panel.js b/src/extensions/default/Git/src/Panel.js index 0311f9cf29..63fe59f1f0 100644 --- a/src/extensions/default/Git/src/Panel.js +++ b/src/extensions/default/Git/src/Panel.js @@ -1050,15 +1050,23 @@ define(function (require, exports) { lastCheckOneClicked = file; + let stagePromise; if (isChecked) { - Git.stage(file, status === Git.FILE_STATUS.DELETED).then(function () { - Git.status(); + stagePromise = Git.stage(file, status === Git.FILE_STATUS.DELETED).then(function () { + return Git.status(); }); } else { - Git.unstage(file).then(function () { - Git.status(); + stagePromise = Git.unstage(file).then(function () { + return Git.status(); }); } + stagePromise.catch((err)=>{ + ErrorHandler.showError(err, Strings.ERROR_STAGE_FAILED, { + dontStripError: true, + errorMetric: "stageOne", + useNotification: true + }); + }); }) .on("dblclick", ".check-one", function (e) { e.stopPropagation(); @@ -1244,9 +1252,19 @@ define(function (require, exports) { return Git.stageAll().then(function () { return Git.status(); }).catch((err)=>{ - console.error(err); - // rethrowing with stripped git error details as it may have sensitive info - throw new Error("Error stage all by checkbox in git panel.js. this should not have happened"); + // this usually happens hwen a git index is locked Eg. error. + // Error: Error: fatal: Unable to create 'E:/.../test-git/.git/index.lock': File exists. + // + // Another git process seems to be running in this repository, e.g. + // an editor opened by 'git commit'. Please make sure all processes + // are terminated then try again. If it still fails, a git process + // may have crashed in this repository earlier: + // remove the file manually to continue. + ErrorHandler.showError(err, Strings.ERROR_STAGE_FAILED, { + dontStripError: true, + errorMetric: "stageAll", + useNotification: true + }); }); } return Git.resetIndex().then(function () { diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index d37504a04e..355d2c9774 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -1513,6 +1513,7 @@ define({ "ERROR_CREATE_TAG": "Create tag failed", "ERROR_CODE_INSPECTION_FAILED": "CodeInspection.inspectFile failed to execute for file", "ERROR_CANT_GET_STAGED_DIFF": "Cant get diff for staged files", + "ERROR_STAGE_FAILED": "Failed to stage files in Git", "ERROR_GIT_COMMIT_FAILED": "Git Commit failed", "ERROR_GIT_BLAME_FAILED": "Git Blame failed", "ERROR_GIT_DIFF_FAILED": "Git Diff failed", diff --git a/src/styles/brackets.less b/src/styles/brackets.less index c93d9784d0..bb44649381 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -3258,6 +3258,13 @@ label input { transform: translateY(0); } +.notification-popup-container.instantOpen { + opacity: 1; + transition: transform 100ms, opacity 100ms; + transition-delay: 100ms; + transform: translateY(0); +} + .notification-popup-container.animateClose { transition: transform 500ms, opacity 500ms; } diff --git a/src/widgets/NotificationUI.js b/src/widgets/NotificationUI.js index b7325124e2..90858caa85 100644 --- a/src/widgets/NotificationUI.js +++ b/src/widgets/NotificationUI.js @@ -127,6 +127,7 @@ define(function (require, exports, module) { endCB && endCB(); } $NotificationPopup.removeClass("animateOpen"); + $NotificationPopup.removeClass("instantOpen"); $NotificationPopup .addClass("animateClose") .one("transitionend", cleanup) @@ -187,10 +188,11 @@ define(function (require, exports, module) { * The template can either be a string or a jQuery object representing a DOM node that is *not* in the current DOM. * * Creating a notification popup + * + * ```js * // note that you can even provide an HTML Element node with * // custom event handlers directly here instead of HTML text. * let notification1 = NotificationUI.createFromTemplate( - * ```js * "
Click me to locate the file in file tree
", "showInfileTree",{ * allowedPlacements: ['top', 'bottom'], * dismissOnClick: false, @@ -345,12 +347,13 @@ define(function (require, exports, module) { * ``` * @param {string} title The title for the notification. * @param {string|Element} template A string template or HTML Element to use as the dialog HTML. - * @param {{dismissOnClick, autoCloseTimeS, toastStyle}} [options] optional, supported + * @param {{dismissOnClick, autoCloseTimeS, toastStyle, instantOpen}} [options] optional, supported * * options are: * * `autoCloseTimeS` - Time in seconds after which the notification should be auto closed. Default is never. * * `dismissOnClick` - when clicked, the notification is closed. Default is true(dismiss). * * `toastStyle` - To style the toast notification for error, warning, info etc. Can be * one of `NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.*` or your own css class name. + * * `instantOpen` - To instantly open the popup without any open animation delays * @return {Notification} Object with a done handler that resolves when the notification closes. * @type {function} */ @@ -378,7 +381,7 @@ define(function (require, exports, module) { // Animate in // Must wait a cycle for the "display: none" to drop out before CSS transitions will work setTimeout(function () { - $NotificationPopup.addClass("animateOpen"); + $NotificationPopup.addClass( options.instantOpen ? "instantOpen" : "animateOpen"); }, 0); if(options.autoCloseTimeS){