Page MenuHomePhabricator

Make error generation code produce an understandable result when called with non-existent arguments
Closed, ResolvedPublic

Description

The module doesn't throw an exception but just goes "that's undefined" which ends up being propagated as an uninformative Z5. We should throw an exception, which would become fixed then.

DoD
Because JavaScript is dang permissive, non-existent exports of the error module are realized as undefined. We do not have tests that exercise this possibility. We should:

  • make tests for what happens when functions are called with undefined, and
  • throw a legible error when that happens.

Related Objects

StatusSubtypeAssignedTask
OpenNone
OpenNone
StalledNone
Resolvedcmassaro
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
DuplicateNone
OpenNone
Resolvedcmassaro
OpenNone
ResolvedNone
ResolvedNone
Resolvedcmassaro
Resolvedcmassaro
ResolvedNone
Resolvedgengh
ResolvedNone
ResolvedNone
Resolvedcmassaro
ResolvedNone
InvalidNone
Invalidgengh
ResolvedDVrandecic
ResolvedDVrandecic
ResolvedDVrandecic
ResolvedSimoneThisDot
ResolvedSimoneThisDot
ResolvedJKieserman
ResolvedNone
ResolvedNone
ResolvedJdforrester-WMF
ResolvedNone
ResolvedNone
DeclinedNone
ResolvedSimoneThisDot
ResolvedSimoneThisDot
Resolvedcmassaro
Resolvedcmassaro
ResolvedSimoneThisDot
ResolvedSimoneThisDot
Resolvedcmassaro
Resolvedcmassaro
ResolvedBUG REPORTJdforrester-WMF
ResolvedSimoneThisDot
Resolvedgengh
Resolvedgengh
Resolvedcmassaro
Resolvedcmassaro
ResolvedJdforrester-WMF
ResolvedDVrandecic
ResolvedDVrandecic
ResolvedSimoneThisDot
ResolvedJdforrester-WMF
ResolvedDVrandecic
ResolvedNone
ResolvedNone
Resolvedcmassaro
ResolvedDVrandecic
ResolvedLindsaykwardell
Resolvedarthurlorenzi
ResolvedNone
InvalidNone
Resolvedcmassaro
Resolvedcmassaro
Resolvedcmassaro
Resolvedcmassaro
Resolvedcmassaro
ResolvedJdforrester-WMF
Resolvedcmassaro
Resolvedcmassaro
Resolvedcmassaro
ResolvedNone
ResolvedSimoneThisDot
ResolvedSimoneThisDot
ResolvedSimoneThisDot
ResolvedJdforrester-WMF
Resolvedcmassaro
ResolvedJdforrester-WMF
Resolvedcmassaro
Resolvedcmassaro
ResolvedNone
ResolvedDVrandecic
Resolvedarthurlorenzi
ResolvedAdesojiThisDot
Resolvedgengh
Resolvedgengh
Resolvedgengh
Resolvedcmassaro
Resolvedcmassaro
Resolvedgengh

Event Timeline

DVrandecic updated the task description. (Show Details)
Jdforrester-WMF renamed this task from Error Generation Code Produces Weird Results When Called with Nonexistent Arguments to Make error generation code produce an understandable result when called with non-existent arguments.Nov 3 2021, 4:27 PM

Because JavaScript is dang permissive, non-existent exports of the error module are realized as undefined. We do not have tests that exercise this possibility. We should 1) make tests for what happens when functions are called with undefined and 2) throw a legible error when that happens.

Hey @gengh, are you still planning to work on this?

gengh changed the task status from Open to In Progress.Mar 7 2022, 4:37 PM

To clarify this patch, are we talking about the errorFormatter.js methods? For example, the main error formatting method createZErrorInstance is passed an errorType ZID and an object with the error creation arguments. If these are not present, the createGenericError method returns something like this, for example in this "key value not wellformed" error:

{
  Z1K1: {
    Z1K1: { Z1K1: 'Z9', Z9K1: 'Z7' },
    Z7K1: { Z1K1: 'Z9', Z9K1: 'Z885' },
    Z885K1: { Z1K1: 'Z9', Z9K1: 'Z526' }
  },
  K1: undefined,
  K2: undefined
}

If this is what this patch is about, should we really raise an error, or generate an error with only the available keys? So, in this case it would be rather a poorly comprehensible error:

{
  Z1K1: {
    Z1K1: { Z1K1: 'Z9', Z9K1: 'Z7' },
    Z7K1: { Z1K1: 'Z9', Z9K1: 'Z885' },
    Z885K1: { Z1K1: 'Z9', Z9K1: 'Z526' }
  }
}

Because this is saying that there's a not wellformed error but we don't know what's happening nor where. But if only one of the arguments were present, we could know the type of error and maybe in what key the error is present.

To summarize:

  • Raising exceptions might be too drastic, as we might discard possibly returnable errors that give at least *some* information of the issue, rather than a "Something went wrong" message.
  • The alternative is to create the error instead, with whatever keys are present, which will at least have information of the "type of error", probably also in the context of a whole error tree.

Change 769417 had a related patch set uploaded (by Genoveva Galarza; author: Genoveva Galarza):

[mediawiki/services/function-schemata@master] Add argument checks to error generation main method

https://gerrit.wikimedia.org/r/769417

Change 769417 merged by jenkins-bot:

[mediawiki/services/function-schemata@master] Add argument checks to error generation main method

https://gerrit.wikimedia.org/r/769417

Change 773496 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/services/function-orchestrator@master] Update function-schemata sub-module to HEAD (e98f603)

https://gerrit.wikimedia.org/r/773496

Change 773497 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/services/function-evaluator@master] Update function-schemata sub-module to HEAD (e98f603)

https://gerrit.wikimedia.org/r/773497

Change 773498 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/extensions/WikiLambda@master] Update function-schemata sub-module to HEAD (e98f603)

https://gerrit.wikimedia.org/r/773498

Change 773508 had a related patch set uploaded (by Jforrester; author: Jforrester):

[mediawiki/services/function-orchestrator@master] [WIP] Update function-schemata sub-module to non-HEAD (993e327)

https://gerrit.wikimedia.org/r/773508

Change 773833 had a related patch set uploaded (by Jforrester; author: Genoveva Galarza):

[mediawiki/services/function-orchestrator@master] Update function-schemata sub-module to non-HEAD (993e327) and make pass

https://gerrit.wikimedia.org/r/773833

Change 773508 abandoned by Jforrester:

[mediawiki/services/function-orchestrator@master] [WIP] Update function-schemata sub-module to non-HEAD (993e327)

Reason:

Squashed into Geno's patch.

https://gerrit.wikimedia.org/r/773508

Change 773833 merged by jenkins-bot:

[mediawiki/services/function-orchestrator@master] Update function-schemata sub-module to non-HEAD (993e327) and make pass

https://gerrit.wikimedia.org/r/773833

Change 773498 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Update function-schemata sub-module to HEAD (e98f603)

https://gerrit.wikimedia.org/r/773498

Change 773497 merged by jenkins-bot:

[mediawiki/services/function-evaluator@master] Update function-schemata sub-module to HEAD (e98f603)

https://gerrit.wikimedia.org/r/773497