Page MenuHomePhabricator
Create Task
Maniphest T314927

Do not suppress root cause of errors
Closed, ResolvedPublic
Actions

Description

Don't know if this has been reported elsewhere already but I couldn't find an equivalent.

Currently some root cause of errors are suppressed and a new error is returned instead.

  • For example this code returns "Could not dereference Z7K1", but that is not very helpful. The underlying error "Generic type function did not return a Z4: {"Z1K1":"Z9","Z9K1":"Z8"}" is much more helpful.

Returning the underlying error, possibly while wrapping it in another error, would be much better. Currently the only way to retrieve this information is to add ad-hoc log messages in the orchestrator while debugging.

Update: the specific instance of this problem linked in this code above has been fixed.

  • Should also examine all other instances of responseEnvelopeContainsError to see if there are any other errors that get discarded. (Note: containsError got renamed to responseEnvelopeContainsError.)

In the following cases, consider whether the result from ValidationStatus.getZ5() should be encapsulated:

  • if ( !validatesAsBoolean( result.Z22K1.asJSON() ).isValid() ) { (builtins.js, line 438)
  • if ( !validatesAsBoolean( headEqualityZ22.Z22K1.asJSON() ).isValid() ) (builtins.js, line 524)

Details

SubjectRepoBranchLines +/-
execute.getArgumentStates: Add inline doc comments with the TODOmediawiki/services/function-orchestratormaster+2 -0
Customize query in gerrit

Related Objects
Search...

Event Timeline

• AAssaf created this task.Aug 10 2022, 10:47 AM
• AAssaf mentioned this in T314926: Generic types are not fully evaluated, wrongly returning errors.
DVrandecic added a project: function-orchestrator.Aug 16 2022, 6:42 PM
DVrandecic moved this task from To triage to Phase θ – Throttling on the Abstract Wikipedia team board.
DVrandecic edited projects, added Abstract Wikipedia team (Phase θ – Throttling); removed Abstract Wikipedia team.
Jdforrester-WMF added a parent task: T321113: Audit and update creation and manipulation of errors in the orchestrator and evaluator.Oct 18 2022, 6:51 PM
Jdforrester-WMF moved this task from Incoming to Ready: Errors on the Abstract Wikipedia team (Phase θ – Throttling) board.Oct 25 2022, 6:40 PM
Jdforrester-WMF added a subtask: T321389: Orchestrator does not pass down error to PHP layer.Dec 7 2022, 9:57 PM
DMartin-WMF closed subtask T321389: Orchestrator does not pass down error to PHP layer as Resolved.Jan 11 2023, 7:39 PM
Jdforrester-WMF triaged this task as High priority.Jan 11 2023, 8:16 PM
gerritbot added a comment.Mar 30 2023, 4:25 PM

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

[mediawiki/services/function-orchestrator@master] execute.getArgumentStates: Add inline doc comments with the TODO

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

gerritbot added a project: Patch-For-Review.Mar 30 2023, 4:25 PM
gerritbot added a comment.Mar 30 2023, 7:31 PM

Change 904573 merged by jenkins-bot:

[mediawiki/services/function-orchestrator@master] execute.getArgumentStates: Add inline doc comments with the TODO

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

Jdforrester-WMF mentioned this in rMSFO9373dc0b8567: execute.getArgumentStates: Add inline doc comments with the TODO.Mar 30 2023, 7:31 PM
Maintenance_bot removed a project: Patch-For-Review.Mar 30 2023, 8:10 PM
Jdforrester-WMF moved this task from Phase θ – Throttling to To triage: Nice-to-haves on the Abstract Wikipedia team board.Jun 16 2023, 8:59 AM
Jdforrester-WMF edited projects, added Abstract Wikipedia team; removed Abstract Wikipedia team (Phase θ – Throttling).
CodeReviewBot added a project: Patch-For-Review.Jul 24 2023, 1:59 AM

apine opened https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-orchestrator/-/merge_requests/51

Remove static validation and ensure that all validator functions are pass-throughs.

CodeReviewBot added a comment.Jul 24 2023, 2:07 PM

jforrester merged https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-orchestrator/-/merge_requests/51

Remove static validation and ensure that all validator functions are pass-throughs.

Maintenance_bot removed a project: Patch-For-Review.Jul 24 2023, 2:12 PM
Jdforrester-WMF moved this task from To triage: Nice-to-haves to Backlog on the Abstract Wikipedia team board.Sep 25 2023, 5:43 PM
cmassaro mentioned this in T321117: Audit our code in the orchestrator and evaluator to ensure we always encapsulate and re-throw rather than re-create errors.Nov 2 2023, 11:49 AM
cmassaro added a parent task: T342656: Consider Creating a Way to Debug Native Code Functions.
Jdforrester-WMF merged a task: T321117: Audit our code in the orchestrator and evaluator to ensure we always encapsulate and re-throw rather than re-create errors.Nov 6 2023, 11:06 PM
Jdforrester-WMF added subscribers: Jdforrester-WMF, cmassaro.
DMartin-WMF updated the task description. (Show Details)Nov 8 2023, 5:05 PM
DMartin-WMF updated the task description. (Show Details)Nov 9 2023, 12:48 AM
DMartin-WMF updated the task description. (Show Details)Nov 10 2023, 5:56 PM
DMartin-WMF updated the task description. (Show Details)Nov 10 2023, 9:32 PM
DMartin-WMF subscribed.Nov 10 2023, 9:37 PM

Regarding these 2 cases:

  • if ( !validatesAsBoolean( result.Z22K1.asJSON() ).isValid() ) { (builtins.js, line 438)
  • if ( !validatesAsBoolean( headEqualityZ22.Z22K1.asJSON() ).isValid() ) (builtins.js, line 524)

even though we are not returning the Z5 from the ValidationStatus returned from validatesAsBoolean, that should be okay in these cases. The existing use of Z516 / Argument value error works well to make it clear what the problem is.

DMartin-WMF updated the task description. (Show Details)Nov 11 2023, 1:07 PM
cmassaro removed a parent task: T342656: Consider Creating a Way to Debug Native Code Functions.Nov 16 2023, 9:58 PM
Jdforrester-WMF changed the task status from Open to In Progress.Nov 20 2023, 7:16 PM
Jdforrester-WMF assigned this task to DMartin-WMF.
Jdforrester-WMF moved this task from Backlog to In Progress on the Abstract Wikipedia team board.

David to do final check for creation of new envelopes where one already exists, and then declare Resolved.

DMartin-WMF closed this task as Resolved.Nov 27 2023, 5:21 AM

I had one last look through the code, everywhere that makeMappedResultEnvelope appears; didn't find any other places where an error is discarded.

Jdforrester-WMF moved this task from In Progress to Verify in production on the Abstract Wikipedia team board.Jan 22 2024, 8:04 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits