-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Fix reduce bug #1578
Fix reduce bug #1578
Conversation
Fix the bug where we get this uncaught error: > TypeError: reduce of empty array with no initial value
Thanks for your fix Jacob! Do you maybe have a (minimal) JSON Schema example that triggers this bug? I can setup a test if for that or explain it to you. |
@jpage-godaddy did you see my comment of last week? |
@josdejong I had missed that. Our schema is rather large; here I've boiled it down somewhat. The error occurs when the "values[].rule.type" property of the "rule" objects doesn't match a valid option, for example when we hit this in the middle of typing the word "equals". {
"values": [
{ "value": false, "isDefault": true },
{ "value": true, "rule": { "type": "e" } }
]
} Here's the reduced schema {
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Ruleset",
"description": "Stores a rule for evaluation",
"type": "object",
"allOf": [
{
"if": {
"required": [
"value"
]
},
"then": {
"$ref": "#/definitions/StaticRule"
}
},
{
"if": {
"required": [
"values"
]
},
"then": {
"$ref": "#/definitions/ConditionalRuleset"
}
}
],
"oneOf": [
{
"required": [
"value"
]
},
{
"required": [
"values"
]
}
],
"definitions": {
"ConditionalRule": {
"description": "A rule containing the criteria for returning a single value.",
"type": "object",
"if": {
"required": [
"rule"
]
},
"then": {
"properties": {
"rule": {
"description": "A type used to determine the deserialization of a rule (and to avoid limitations with dyn trait objects).",
"type": "object",
"allOf": [
{
"if": {
"properties": {
"type": {
"const": "equals"
}
}
},
"then": {
"description": "A rule evaluating the equality of the value of a named JSON property to specified JSON value.\n\nCompares JSON strings, numbers, and booleans. Strings are compared with case-sensitive equality.\n\nReturns `false` if the types are not matching. Returns `false` for `null`, objects, or arrays.",
"type": "object",
"required": [
"value",
"variableName"
],
"properties": {
"value": {
"description": "What the variable data provided from the `RuleEvaluationCriteria` should be equal to."
},
"variableName": {
"description": "The name of the variable in the provided `RuleEvaluationCriteria` to compare.",
"type": "string"
}
}
}
},
{
"if": {
"properties": {
"type": {
"const": "not"
}
}
},
"then": {
"description": "A rule negating the evaluation of another rule.",
"type": "object",
"required": [
"rule"
],
"properties": {
"rule": {
"description": "The rule to be negated.",
"allOf": [
{
"$ref": "#/definitions/RuleType"
}
]
}
}
}
}
],
"required": [
"type"
],
"properties": {
"type": {
"enum": [
"equals",
"not"
]
}
}
}
}
},
"required": [
"value"
],
"properties": {
"isDefault": {
"description": "When true, this rule's value should be prioritized if multiple rules match, and it should also be used as a default if no other rule qualifies.",
"type": [
"boolean",
"null"
]
},
"rule": {
"description": "Qualification(s) to evaluate as true in order for this rule to return its value.",
"type": [
"object",
"null"
]
},
"value": {
"description": "A value for the rule if evaluation criteria are applicable."
}
}
},
"ConditionalRuleset": {
"type": "object",
"required": [
"values"
],
"properties": {
"values": {
"description": "An ordered subset of possible values to evaluate under this rule. Use \"values\" to create a set of conditions for the rules engine to evaluate. For example, value 1 could be 'if str_value equals \"my-string\", return true'; And value 2 could be 'else, return false'.",
"type": "array",
"items": {
"$ref": "#/definitions/ConditionalRule"
}
}
}
},
"RuleType": {
"description": "A type used to determine the deserialization of a rule (and to avoid limitations with dyn trait objects).",
"type": "object",
"allOf": [
{
"if": {
"properties": {
"type": {
"const": "equals"
}
}
},
"then": {
"description": "A rule evaluating the equality of the value of a named JSON property to specified JSON value.\n\nCompares JSON strings, numbers, and booleans. Strings are compared with case-sensitive equality.\n\nReturns `false` if the types are not matching. Returns `false` for `null`, objects, or arrays.",
"type": "object",
"required": [
"value",
"variableName"
],
"properties": {
"value": {
"description": "What the variable data provided from the `RuleEvaluationCriteria` should be equal to."
},
"variableName": {
"description": "The name of the variable in the provided `RuleEvaluationCriteria` to compare.",
"type": "string"
}
}
}
},
{
"if": {
"properties": {
"type": {
"const": "not"
}
}
},
"then": {
"description": "A rule negating the evaluation of another rule.",
"type": "object",
"required": [
"rule"
],
"properties": {
"rule": {
"description": "The rule to be negated.",
"allOf": [
{
"$ref": "#/definitions/RuleType"
}
]
}
}
}
}
],
"required": [
"type"
],
"properties": {
"type": {
"enum": [
"equals",
"not"
]
}
}
},
"StaticRule": {
"type": "object",
"required": [
"value"
],
"properties": {
"value": {
"description": "The static value to assign a setting."
}
}
},
"value": {
"required": [
"value"
]
},
"values": {
"required": [
"values"
]
}
}
} So perhaps it's a bug with |
Thanks, I can reproduce the issue with your example schema and JSON. The fix of The issue could originate in |
To me it appears that this should be the correct fix; the error seems to stem from a case where there's no suggestions available, when My main motivation for the pull request is to keep uncaught errors from popping up in our application, disrupting our user experience. If there's a bug where the schema isn't deriving suggestions properly when it should be, that's probably a separate issue to be addressed. Mind if we get the errors fixed and tackle ways to improve the schema parsing separately? |
Ah, ok that makes sense. So:
|
The fix is published now in |
Fix the bug where we get this uncaught error:
I'm not sure yet how to build construct a unit test to demonstrate the issue, but it is occurring for our JSON schema. At this line, when
currentSuggestions
is an empty object{}
,Object.keys
is an empty array, and therefore without having an initial value for the call to.reduce(...)
it'll bomb out.