Repository: OpenRefine/OpenRefine (java)

Issue:
Stored Error functionality causes slient errors and makes Transforms appear successful when not
I wanted OpenRefine to actually store the error, since I chose the radio option "On error: store error" in the Custom text transform on a column of Strings which I am trying to convert to a Number, but it does not store the error in any cell values.  Furthermore, OpenRefine does not report anything about an error and is silently broken when applying a Number transformation to a String.

### To Reproduce
Steps to reproduce the behavior:
1. Edit Cells -> Transform
2. Apply GREL to a String value of "12" with `value.toNumber().toString("%.4f")`
3. Choose "store error" at the bottom of the Custom text transform dialog
4. Click OK on dialog

### Current Results
If I Create a custom text facet with the GREL in step 2, then I get a facet with values changed, and those that had an error are categorized as `(error)`. Great!  But the problem is if I don't do a facet, and instead choose to do the Edit Cells -> Transform and choose the radio option **On error** to "store error" and click OK, then the columns' value does not store the java error, or anything, and in fact, doesn't run the transformation at all on any of the column values - it's silently broken without telling the user that there was some critical error behind the scenes and it couldn't run the entire transformation.



### Expected Behavior
I expected that if I choose "store error" that in my example project, `13.544897` would be changed to `13.5449` and `11.45` changed to `11.4500` and that `12` changed to `Error: java.util.IllegalFormatConversionException: f != java.lang.Long`

I should have been notified that there was an error applying the transformation! At least a banner!  And not appear as if it was successful by NOT showing a banner.  We should have throw a red error banner - "No Op - Transform had an error"

If I chose to "store error", I'd expect OpenRefine to store the error text strings/messages like the small java error string, so that I can use Facets to filter on those that were flagged as an error during transform and fix them or rerun a slightly different GREL on just those rows.  Better yet is a full error context (error flag, error message, original value) object stored along with the cell's value such as suggested in #495 

But for now, to fix this issue itself, we should at least fix this silent error to not be silent on the UI and that when a user clicks on "store error" and runs a transformation, which doesn't actually run on any cells at all, but makes it appear to have been applied successfully.  When a project has many rows, and not only a few, this can accidentally appear to a user that the transformation was successful because no banner, or error, or anything alarming of "unsuccessful" was shown in the UI.

### Screenshots
![image](https://github.com/user-attachments/assets/2ee82c53-fa32-48d5-81ba-c591b9750a52)

### Versions<!--  (please complete the following information)-->
 - Operating System: Windows 11 Pro
 - Browser Version: Edge latest
 - JRE or JDK Version: openjdk 21
 - OpenRefine: 3.9-SNAPSHOT [TRUNK]

### Datasets
<!-- If you are allowed and are OK with making your data public, it would be awesome if you can include or attach the data causing the issue or a URL pointing to where the data is.
If you are concerned about keeping your data private, you can share it selectively by email to developers who work on the issue -->
My test project:
[Clipboard.openrefine-TEST.tar.gz](https://github.com/user-attachments/files/17528695/Clipboard.openrefine-TEST.tar.gz)

### Additional context
**Labeling as Critical Priority because OpenRefine is giving false impressions to a user that something was successful when it actually was not.**
related #495


Pull request:
Broaden illegal format exception caught. Fixes #6943
Fixes #6943

Changes proposed in this pull request:
- Broaden illegal format exception caught. 

Patch:
diff --git a/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.js b/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.js
index 83310818b..dfd0fe056 100644
--- a/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.js
+++ b/main/webapp/modules/core/scripts/dialogs/column-reordering-dialog.js
@@ -86,6 +86,7 @@ ColumnReorderingDialog.prototype._dismiss = function() {
 };
 
 ColumnReorderingDialog.prototype._commit = function() {
+    var self = this;
     var columnNames = this._elmts.columnContainer.find('div').map(function() { return this.getAttribute("column"); }).get();
     
     Refine.postCoreProcess(
@@ -93,8 +94,9 @@ ColumnReorderingDialog.prototype._commit = function() {
         null,
         { "columnNames" : JSON.stringify(columnNames) }, 
         { modelsChanged: true, rowIdsPreserved: true }, // TODO could add recordIdsPreserved: true if the record key column did not change
-        { includeEngine: false }
+        { 
+          includeEngine: false,
+          onDone: function() { self._dismiss(); }
+        }
     );
-    
-    this._dismiss();
 };
diff --git a/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.js b/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.js
index 1632078c7..d66f60632 100644
--- a/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.js
+++ b/main/webapp/modules/core/scripts/dialogs/common-transform-dialog.js
@@ -106,29 +106,32 @@ commonTransformDialog.prototype._dismiss = function() {
 
 
 commonTransformDialog.prototype._commit = function(expression) {
-      var doTextTransform = function(columnName, expression, onError, repeat, repeatCount) {
+  var self = this;
+  var columnNames = [];
+  this._elmts.columnContainer.find('div').each(function() {
+    if ($(this).find('input[type="checkbox"]')[0].checked) {
+      var name = this.getAttribute('column');
+      columnNames.push(name);
+    }
+  });
+  var doTextTransform = function(index, expression, onError, repeat, repeatCount) {
+    if (index < columnNames.length) {
       Refine.postCoreProcess(
         "text-transform",
         {
-          columnName: columnName, 
+          columnName: columnNames[index], 
           expression: expression, 
           onError: onError,
           repeat: repeat,
           repeatCount: repeatCount
         },
         null,
-        { cellsChanged: true, rowIdsPreserved: true }
+        { cellsChanged: true, rowIdsPreserved: true },
+        { onDone: function() { doTextTransform(index + 1, expression, onError, repeat, repeatCount) } }
       );
-  };
-
-	this._elmts.columnContainer.find('div').each(function() {
-    if ($(this).find('input[type="checkbox"]')[0].checked) {
-      var name = this.getAttribute('column');
-	    doTextTransform(name,expression, "keep-original", false, "");
+    } else {
+      self._dismiss();
     }
-  });
-  
-    
-    this._dismiss();
-    
+  };
+  doTextTransform(0, expression, "keep-original", false, "");
 };
diff --git a/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.js b/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.js
index ca2334e71..55f64b02c 100644
--- a/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.js
+++ b/main/webapp/modules/core/scripts/dialogs/expression-column-dialog.js
@@ -1,5 +1,5 @@
 
-var doTextTransform = function(columnName, expression, onError, repeat, repeatCount) {
+var doTextTransform = function(columnName, expression, onError, repeat, repeatCount, onDone) {
     Refine.postCoreProcess(
       "text-transform",
       {
@@ -10,7 +10,8 @@ var doTextTransform = function(columnName, expression, onError, repeat, repeatCo
         repeatCount: repeatCount
       },
       null,
-      { cellsChanged: true, rowIdsPreserved: true }
+      { cellsChanged: true, rowIdsPreserved: true },
+      { onDone }
     );
 };
 
@@ -87,7 +88,6 @@ ExpressionColumnDialog.prototype._dismiss = function() {
 
 ExpressionColumnDialog.prototype._transform = function() {
   this._postSelect();
-  this._dismiss();
 };
 
 ExpressionColumnDialog.prototype._postSelect = function() {
@@ -96,7 +96,7 @@ ExpressionColumnDialog.prototype._postSelect = function() {
     if ($(this).find('input[type="checkbox"]')[0].checked) {
       var name = this.getAttribute('column');
       // alert("doTextTransform on: " + name + "; expression: " + self._expression);
-	  doTextTransform(name, self._expression, self._onError, self._repeat, self._repeatCount)
+	  doTextTransform(name, self._expression, self._onError, self._repeat, self._repeatCount, self._dismiss)
     }
   });
   
diff --git a/main/webapp/modules/core/scripts/facets/list-facet.js b/main/webapp/modules/core/scripts/facets/list-facet.js
index ffcc6e1fd..7fe7567e7 100644
--- a/main/webapp/modules/core/scripts/facets/list-facet.js
+++ b/main/webapp/modules/core/scripts/facets/list-facet.js
@@ -583,8 +583,6 @@ class ListFacet extends Facet {
     var commit = function() {
       var text = elmts.textarea[0].value;
 
-      MenuSystem.dismissAll();
-
       var edit = { to : text };
       if (choice === self._blankChoice) {
         edit.fromBlank = true;
@@ -609,6 +607,7 @@ class ListFacet extends Facet {
         },
         {
           onDone: function(o) {
+            MenuSystem.dismissAll();
             var selection = [];
             var gotSelection = false;
             for (var i = 0; i < self._selection.length; i++) {
diff --git a/main/webapp/modules/core/scripts/util/csrf.js b/main/webapp/modules/core/scripts/util/csrf.js
index d156916d6..a904a523f 100644
--- a/main/webapp/modules/core/scripts/util/csrf.js
+++ b/main/webapp/modules/core/scripts/util/csrf.js
@@ -20,11 +20,7 @@ CSRFUtil.postCSRF = function(url, data, success, dataType, failCallback) {
     return CSRFUtil.wrapCSRF(function(token) {
         var fullData = data || {};
         if (typeof fullData == 'string') {
-            if (fullData.includes('?')) {
-              fullData = fullData + "&" + $.param({csrf_token: token});
-            } else {
-              fullData = fullData + "?" + $.param({csrf_token: token});
-            }
+            fullData = fullData + "&" + $.param({csrf_token: token});
         } else {
             fullData['csrf_token'] = token;
         }
diff --git a/main/webapp/modules/core/scripts/views/data-table/menu-edit-cells.js b/main/webapp/modules/core/scripts/views/data-table/menu-edit-cells.js
index f2b7978b5..584d81d42 100644
--- a/main/webapp/modules/core/scripts/views/data-table/menu-edit-cells.js
+++ b/main/webapp/modules/core/scripts/views/data-table/menu-edit-cells.js
@@ -32,7 +32,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
-  var doTextTransform = function(expression, onError, repeat, repeatCount) {
+  var doTextTransform = function(expression, onError, repeat, repeatCount, onDone) {
     Refine.postCoreProcess(
       "text-transform",
       {
@@ -42,7 +42,8 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
         repeatCount: repeatCount
       },
       { expression: expression },
-      { cellsChanged: true }
+      { cellsChanged: true },
+      { onDone },
     );
   };
 
@@ -72,9 +73,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
         previewWidget.getExpression(true),
         $('input[name="text-transform-dialog-onerror-choice"]:checked')[0].value,
         elmts.repeatCheckbox[0].checked,
-        elmts.repeatCountInput[0].value
+        elmts.repeatCountInput[0].value,
+        dismiss
       );
-      dismiss();
     });
 
     var o = DataTableView.sampleVisibleRows(column);
@@ -285,8 +286,7 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
         text_to_find = '"'+text_to_find+'"';
       }
       expression = 'value.replace('+text_to_find+',"'+replacement_text+'")';
-      doTextTransform(expression, "keep-original", false, "");
-      dismiss();
+      doTextTransform(expression, "keep-original", false, "", dismiss);
     });
   };
 
@@ -383,10 +383,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
         "split-multi-value-cells",
         config,
         null,
-        { rowsChanged: true }
+        { rowsChanged: true },
+        { onDone: dismiss }
       );
-
-      dismiss();
     });
   };
 
@@ -701,9 +700,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
         "key-value-columnize",
         config,
         null,
-        { modelsChanged: true }
+        { modelsChanged: true },
+        { onDone: dismiss }
       );
-      dismiss();
     });
 
     var valueColumnIndex = -1;
diff --git a/main/webapp/modules/core/scripts/views/data-table/menu-edit-column.js b/main/webapp/modules/core/scripts/views/data-table/menu-edit-column.js
index 9c9dec8fa..a27988a18 100644
--- a/main/webapp/modules/core/scripts/views/data-table/menu-edit-column.js
+++ b/main/webapp/modules/core/scripts/views/data-table/menu-edit-column.js
@@ -166,9 +166,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
           httpHeaders: JSON.stringify(elmts.setHttpHeadersContainer.find("input").serializeArray())
         },
         null,
-        { modelsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true }
+        { modelsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true },
+        { onDone: dismiss }
       );
-      dismiss();
     });
   };
 
@@ -358,9 +358,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
         "split-column", 
         config,
         null,
-        { modelsChanged: true }
+        { modelsChanged: true },
+        { onDone: dismiss }
       );
-      dismiss();
     });
   }; 
   
@@ -399,7 +399,6 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
       // function called in a callback
       var deleteColumns = function() {
         if (deleteJoinedColumns) {
-          console.log (theProject);
           var columnsToKeep = theProject.columnModel.columns
           .map (function (col) {return col.name;})
           .filter (function(colName) {
@@ -466,7 +465,7 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
           },
           { expression: expression },
           { modelsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true },
-          { onFinallyDone: deleteColumns}
+          { onDone: dismiss, onFinallyDone: deleteColumns}
         );
       } 
       else {
@@ -476,7 +475,7 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
             onError,
             repeat,
             repeatCount,
-            { onFinallyDone: deleteColumns});
+            { onDone: dismiss, onFinallyDone: deleteColumns});
       }
     };
     // core of doJoinColumn
@@ -549,7 +548,6 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
       });
     elmts.okButton.on('click',function() {
       transform();
-      dismiss();
     });
     elmts.cancelButton.on('click',function() {
       dismiss();
diff --git a/main/webapp/modules/core/scripts/views/data-table/menu-reconcile.js b/main/webapp/modules/core/scripts/views/data-table/menu-reconcile.js
index 54f649774..9b5d0cd1f 100644
--- a/main/webapp/modules/core/scripts/views/data-table/menu-reconcile.js
+++ b/main/webapp/modules/core/scripts/views/data-table/menu-reconcile.js
@@ -221,10 +221,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
             schemaSpace: service.schemaSpace
         },
         null,
-        { cellsChanged: true, columnStatsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true }
+        { cellsChanged: true, columnStatsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true },
+        { onDone: function() { DialogSystem.dismissUntil(level - 1); } }
         );
-
-        DialogSystem.dismissUntil(level - 1);
     });
 
     $('<button class="button"></button>').text($.i18n('core-buttons/cancel')).on('click',function() {
@@ -395,9 +394,9 @@ DataTableColumnHeaderUI.extendMenu(function(column, columnHeaderUI, menu) {
           "recon-copy-across-columns", 
           null,
           config,
-          { rowsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true }
+          { rowsChanged: true, rowIdsPreserved: true, recordIdsPreserved: true },
+          { onDone: dismiss }
         );
-        dismiss();
       }
     });
   };
diff --git a/modules/grel/src/main/java/com/google/refine/expr/functions/ToString.java b/modules/grel/src/main/java/com/google/refine/expr/functions/ToString.java
index f4b11b037..bac23d0c1 100644
--- a/modules/grel/src/main/java/com/google/refine/expr/functions/ToString.java
+++ b/modules/grel/src/main/java/com/google/refine/expr/functions/ToString.java
@@ -35,8 +35,8 @@ package com.google.refine.expr.functions;
 
 import java.time.OffsetDateTime;
 import java.time.format.DateTimeFormatter;
+import java.util.IllegalFormatException;
 import java.util.Properties;
-import java.util.UnknownFormatConversionException;
 
 import com.google.refine.expr.EvalError;
 import com.google.refine.grel.ControlFunctionRegistry;
@@ -59,7 +59,7 @@ public class ToString implements Function {
                 } else if (o1 instanceof Number) {
                     try {
                         return String.format((String) o2, o1);
-                    } catch (UnknownFormatConversionException e) {
+                    } catch (IllegalFormatException e) {
                         return new EvalError(EvalErrorMessage.unknown_format_conversion(e.getMessage()));
                     }
                 }


Test patch:
diff --git a/main/tests/cypress/cypress/e2e/project/undo_redo/warn_for_history_deletion.cy.js b/main/tests/cypress/cypress/e2e/project/undo_redo/warn_for_history_deletion.cy.js
index 6c61ebdf9..8faa02bd5 100644
--- a/main/tests/cypress/cypress/e2e/project/undo_redo/warn_for_history_deletion.cy.js
+++ b/main/tests/cypress/cypress/e2e/project/undo_redo/warn_for_history_deletion.cy.js
@@ -83,4 +83,50 @@ describe(__filename, function () {
     // There are no future changes anymore, since they have been deleted
     cy.get('.history-panel-body .history-future a').should('not.exist');
   });
+
+  it('Apply a text transform after undoing some changes', function () {
+    cy.loadAndVisitProject('food.mini');
+
+    // delete NDB_No
+    cy.deleteColumn('NDB_No');
+    cy.get('#or-proj-undoRedo').should('to.contain', '1 / 1');
+    cy.get('.history-panel-body .history-now').should(
+      'to.contain',
+      'Remove column NDB_No'
+    );
+
+    // Open the Undo/Redo panel
+    cy.get('#or-proj-undoRedo').click();
+
+    // undo the change
+    cy.get(
+      '.history-panel-body .history-past a.history-entry:first-of-type'
+    ).click();
+    cy.get('.history-panel-body .history-now').should(
+      'to.contain',
+      'Create project'
+    );
+    cy.get('.history-panel-body .history-future').should(
+      'to.contain',
+      'Remove column NDB_No'
+    );
+
+    cy.columnActionClick('Shrt_Desc', ['Edit cells', 'Transform']);
+
+    cy.typeExpression('value.replace("E","e")');
+    cy.get('.dialog-footer .button-primary').click();
+
+    // ensure we get a dialog warning us for deletion of changes
+    cy.get('.dialog-header').should('to.contain', 'Confirm deletion of project history');
+    // confirm the dialog   
+    cy.get('.dialog-footer .button-primary').contains('Apply anyway').click();
+
+    // ensure value has been changed in the grid
+    cy.get('.menu-container.data-table-cell-editor').should('not.exist');
+    cy.assertCellEquals(1, 'Shrt_Desc', 'BUTTeR,WHIPPeD,WITH SALT');
+
+    // There are no future changes anymore, since they have been deleted
+    cy.get('.history-panel-body .history-future a').should('not.exist');
+  });
+
 });
diff --git a/modules/grel/src/test/java/com/google/refine/expr/functions/ToStringTests.java b/modules/grel/src/test/java/com/google/refine/expr/functions/ToStringTests.java
index 1b7974011..a38b7549b 100644
--- a/modules/grel/src/test/java/com/google/refine/expr/functions/ToStringTests.java
+++ b/modules/grel/src/test/java/com/google/refine/expr/functions/ToStringTests.java
@@ -59,6 +59,7 @@ public class ToStringTests extends GrelTestBase {
         assertTrue(invoke("toString", 100L, "%") instanceof EvalError);
         assertTrue(invoke("toString", 100L, "%.") instanceof EvalError);
         assertTrue(invoke("toString", 100L, "%0") instanceof EvalError);
+        assertTrue(invoke("toString", 12L, "%2.2f") instanceof EvalError);
 
         // test with large number
         assertEquals(invoke("toString", 1000000000000000000L, "%d"), "1000000000000000000");