Skip to content

Handle missing executor in NodeJS #528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sven-oly wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Handle missing executor in NodeJS #528

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c82fe0a
Change to elif
sven-oly Aug 29, 2025
c702757
Merge remote-tracking branch 'upstream/main'
sven-oly Aug 29, 2025
3dc28a5
Merge remote-tracking branch 'upstream/main'
sven-oly Aug 29, 2025
bda88c6
Merge remote-tracking branch 'upstream/main'
sven-oly Sep 3, 2025
ae3797a
Adding schema for missing label info
sven-oly Sep 3, 2025
4aee413
Return error in NodeJS when executor can't handle the test.
sven-oly Sep 8, 2025
218d52a
Update testplan for missing labels
sven-oly Oct 2, 2025
9cf3a0e
Fix typo
sven-oly Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion executors/node/executor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,10 @@ rl.on('line', function(line) {
if (test_type == "segmenter") {
outputLine = segmenter.testSegmenter(parsedJson);
} else {
outputLine = {'error': 'unknown test type',
// UNSUPPORTED TEST TYPE!
outputLine = {'label': 'UNKNOWN',
'error': 'unknown test type',
'error_detail': 'Requested unsupported test type'
'test_type': test_type,
'unsupported_test': test_type};
Comment thread
sven-oly marked this conversation as resolved.
}
Expand Down
6 changes: 5 additions & 1 deletion schema/collation/test_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,11 @@
"warning": {
"description": "Present when data has a problem",
"type": "string"
}
},
"hash_id": {
"description": "Has value of the JSON string omtting label value",
Comment thread
sven-oly marked this conversation as resolved.
"type": "integer"
}
Comment on lines +115 to +118
Copy link
Copy Markdown
Member

@sffc sffc Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I don't see hash_id used anywhere in this PR outside the schema. Why do you need it?

},
"required": [
"label",
Expand Down
4 changes: 4 additions & 0 deletions schema/lang_names/test_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
"locale_label": {
"description": "locale tag of the language being described ",
"type": "string"
},
"hash_id": {
"description": "Has value of the JSON string omtting label value",
Comment thread
sven-oly marked this conversation as resolved.
"type": "integer"
}
},
"additionalProperties": false
Expand Down
4 changes: 4 additions & 0 deletions schema/likely_subtags/test_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@
"option": {
"description": "Type of processing requested",
"enum": ["maximize", "minimize", "minimizeFavorScript", "minimizeFavorRegion"]
},
"hash_id": {
"description": "Has value of the JSON string omtting label value",
Comment thread
sven-oly marked this conversation as resolved.
"type": "integer"
}
}
},
Expand Down
4 changes: 4 additions & 0 deletions schema/number_format/test_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@
"default": "auto"
}
}
},
"hash_id": {
"description": "Has value of the JSON string omtting label value",
Comment thread
sven-oly marked this conversation as resolved.
"type": "integer"
}
},
"required": [
Expand Down
6 changes: 4 additions & 2 deletions testdriver/testplan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,11 @@ def send_one_line(self, input_line):
logging.error(' !!!!!! %s' % self.run_error_message)

# Handle problems with decoding errors and other unknowns.
error_result = {'label': 'UNKNOWN',
error_result = {'label': input['label'],
'input_data': input_line,
'error': self.run_error_message
'error': self.run_error_message,
'error_detail': 'severe error from subprocess ' +
result.returncode,
}
Comment on lines +518 to 523
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This block introduces two critical issues that will cause a crash:

  1. NameError: The variable input is used to get a label (input['label']), but it is not defined within this else block's scope. It is only defined in the except block below.
  2. TypeError: result.returncode is an integer. Concatenating it with a string using + will raise a TypeError. It must be converted to a string first, for example, by using str(result.returncode).

I've provided a suggestion that resolves both issues by safely parsing the input to retrieve the label and correctly handling the type conversion.

                label = 'UNKNOWN'
                try:
                    input_json = json.loads(input_line.splitlines()[0])
                    label = input_json.get('label', 'UNKNOWN')
                except (IndexError, json.JSONDecodeError):
                    pass  # Fallback to 'UNKNOWN' if parsing fails
                error_result = {'label': label,
                                'input_data': input_line,
                                'error': self.run_error_message,
                                'error_detail': 'severe error from subprocess ' +
                                    str(result.returncode),
                                }

Copy link
Copy Markdown
Member

@sffc sffc Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please fix

return json.dumps(error_result)
except BaseException as err:
Expand Down
Loading