Copilot commented on code in PR #6288:
URL: https://github.com/apache/texera/pull/6288#discussion_r3553003246
##########
frontend/src/app/workspace/component/result-panel/error-frame/error-frame.component.ts:
##########
@@ -72,11 +72,41 @@ export class ErrorFrameComponent implements OnInit {
errorMessages = errorMessages.filter(err => err.operatorId ===
this.operatorId);
}
this.categoryToErrorMapping = errorMessages.reduce((acc, obj) => {
- const key = obj.type.name;
+ let key = obj.type.name;
+ let message = obj.message;
+ let details = obj.details;
+
+ if (key === "COMPILATION_ERROR") {
+ // key = "WARNING";
+
+ // Strip out common Java exception class names and formatting to make
it more user-friendly
+ const exceptionRegex =
/^\s*(?:[a-zA-Z0-9_]+\.)+[a-zA-Z0-9_]+Exception:\s*/;
+ const requirementFailedRegex = /^\s*requirement failed:\s*/;
+ const genericExceptionRegex = /^\s*Exception:\s*/;
Review Comment:
The PR description says the frontend change is a global catch-all that
strips Java/Scala exception prefixes, but this formatting is currently only
applied when `obj.type.name === "COMPILATION_ERROR"`. If other error categories
can surface raw exception prefixes, they will remain unformatted.
##########
frontend/src/app/workspace/component/result-panel/error-frame/error-frame.component.ts:
##########
@@ -72,11 +72,41 @@ export class ErrorFrameComponent implements OnInit {
errorMessages = errorMessages.filter(err => err.operatorId ===
this.operatorId);
}
this.categoryToErrorMapping = errorMessages.reduce((acc, obj) => {
- const key = obj.type.name;
+ let key = obj.type.name;
+ let message = obj.message;
+ let details = obj.details;
+
+ if (key === "COMPILATION_ERROR") {
+ // key = "WARNING";
+
+ // Strip out common Java exception class names and formatting to make
it more user-friendly
+ const exceptionRegex =
/^\s*(?:[a-zA-Z0-9_]+\.)+[a-zA-Z0-9_]+Exception:\s*/;
+ const requirementFailedRegex = /^\s*requirement failed:\s*/;
+ const genericExceptionRegex = /^\s*Exception:\s*/;
+
+ if (message) {
+ message = message.replace(exceptionRegex, "");
+ message = message.replace(requirementFailedRegex, "");
+ message = message.replace(genericExceptionRegex, "");
+ }
Review Comment:
`renderError()` now rewrites compilation error `message`/`details`, but the
existing unit test only checks component creation. Adding a focused test for
the new prefix-stripping behavior would prevent regressions (e.g., ensuring
`java.lang.IllegalArgumentException: requirement failed: ...` renders as the
human-readable message).
##########
frontend/src/app/workspace/component/result-panel/error-frame/error-frame.component.ts:
##########
@@ -72,11 +72,41 @@ export class ErrorFrameComponent implements OnInit {
errorMessages = errorMessages.filter(err => err.operatorId ===
this.operatorId);
}
this.categoryToErrorMapping = errorMessages.reduce((acc, obj) => {
- const key = obj.type.name;
+ let key = obj.type.name;
+ let message = obj.message;
+ let details = obj.details;
+
+ if (key === "COMPILATION_ERROR") {
+ // key = "WARNING";
+
+ // Strip out common Java exception class names and formatting to make
it more user-friendly
Review Comment:
Remove the leftover commented-out key reassignment and blank line; it’s dead
code and makes the new formatting block harder to read/maintain.
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/sql/SQLSourceOpDesc.scala:
##########
@@ -138,12 +138,30 @@ abstract class SQLSourceOpDesc extends
SourceOperatorDescriptor {
* @return Schema
*/
private def querySchema: Schema = {
- if (
- this.host == null || this.port == null || this.database == null
- || this.table == null || this.username == null || this.password == null
- ) {
- return null
- }
+ require(
+ host != null && host.trim.nonEmpty,
+ s"Please enter a valid host name for the database in the properties
panel."
+ )
+ require(
+ port != null && port.trim.nonEmpty,
+ s"Please enter a valid port for the database in the properties panel."
+ )
+ require(
+ database != null && database.trim.nonEmpty,
+ s"Please enter a valid database name in the properties panel."
+ )
+ require(
+ table != null && table.trim.nonEmpty,
+ s"Please enter a valid table name in the properties panel."
+ )
+ require(
+ username != null && username.trim.nonEmpty,
+ s"Please enter a valid username in the properties panel."
+ )
+ require(
+ password != null,
+ s"Please enter a valid password in the properties panel."
+ )
Review Comment:
Password validation currently only checks for `null`, but the UI can submit
an empty string; that would bypass this guard and fall through to a JDBC
connection attempt, producing a less actionable SQL error. For consistency with
the other fields (and the user-facing message), validate `trim.nonEmpty`.
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/projection/ProjectionOpDesc.scala:
##########
@@ -56,7 +56,7 @@ class ProjectionOpDesc extends MapOpDesc {
.withOutputPorts(operatorInfo.outputPorts)
.withDerivePartition(derivePartition())
.withPropagateSchema(SchemaPropagationFunc(inputSchemas => {
- require(attributes.nonEmpty, "Attributes must not be empty")
+ require(attributes.nonEmpty, "Please select at least 1 attribute to
project.")
Review Comment:
Minor wording: prefer “one” over “1” in user-facing text.
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/arrow/ArrowSourceOpDesc.scala:
##########
@@ -81,8 +94,11 @@ class ArrowSourceOpDesc extends ScanSourceOpDesc {
val arrowSchema: ArrowSchema = reader.getVectorSchemaRoot.getSchema
ArrowUtils.toTexeraSchema(arrowSchema)
}
- .getOrElse {
- throw new IOException("Failed to infer schema from Arrow file.")
- }
+ .recover { case scala.util.control.NonFatal(e) =>
+ throw new RuntimeException(
+ "Failed to read the .arrow file. Please ensure it is a valid Arrow
file.",
+ e
+ )
+ }.get
Review Comment:
Throwing inside `.recover { ... }` is unusual and makes control-flow harder
to follow; it also bypasses the Try-based error pipeline. Prefer returning a
`Failure` (via `recoverWith`) and let `.get` throw, preserving the same
user-facing message and cause.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
