0015356: improve error handling in mc_issue_api.php - MantisBT

ID	Project	Category	View Status	Date Submitted	Last Update
0015356	mantisbt	api soap	public	2013-01-08 13:24	2013-04-06 08:30

Reporter	jeckyll	Assigned To	rombert
Priority	normal	Severity	major	Reproducibility	always
Status	closed	Resolution	fixed
Platform	PHP 5.2.12 under FastCGI / IIS 6	OS	Windows Server 2003
Product Version	1.2.12
Target Version	1.2.13	Fixed in Version	1.2.13

Summary	0015356: improve error handling in mc_issue_api.php
Description	mci_issue_set_custom_fields function returns a soap_fault object in case of something goes wrong. Unfortunately the returned value of this function is never checked Because of that, it is really hard to debug problems related to custom fields update, since these errors are never sent back to the caller. The fix is very straightfoward : simply check the returned value of mci_issue_set_custom_fields before continuing ; for example : function mc_issue_update( $p_username, $p_password, $p_issue_id, $p_issue ) { ... some existing code ... $set_custom_field_error = mci_issue_set_custom_fields( $p_issue_id, $p_issue['custom_fields'], true ); return any error raised by mci_issue_set_custom_fields if ( $set_custom_field_error != null ) return $set_custom_field_error; ... }
Tags	No tags attached.
Attached Files	fix 15356 [SOAP Api].patch (1,144 bytes) diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php index f0c2fcb..b144d34 100644 --- a/api/soap/mc_issue_api.php +++ b/api/soap/mc_issue_api.php @@ -675,7 +675,9 @@ # submit the issue $t_issue_id = $t_bug_data->create(); - mci_issue_set_custom_fields( $t_issue_id, $p_issue['custom_fields'], false ); + $t_set_custom_field_error = mci_issue_set_custom_fields( $t_issue_id, $p_issue['custom_fields'], false ); + if ( $t_set_custom_field_error != null ) return $t_set_custom_field_error; + if ( isset ( $p_issue['monitors'] ) ) mci_issue_set_monitors( $t_issue_id , $t_user_id, $p_issue['monitors'] ); @@ -858,7 +860,9 @@ $t_bug_data->target_version = isset( $p_issue['target_version'] ) ? $p_issue['target_version'] : ''; } - mci_issue_set_custom_fields( $p_issue_id, $p_issue['custom_fields'], true ); + $t_set_custom_field_error = mci_issue_set_custom_fields( $p_issue_id, $p_issue['custom_fields'], true ); + if ( $t_set_custom_field_error != null ) return $t_set_custom_field_error; + if ( isset ( $p_issue['monitors'] ) ) mci_issue_set_monitors( $p_issue_id , $t_user_id, $p_issue['monitors'] ); fix 15356 [SOAP Api].patch (1,144 bytes) fix-15356-SOAP-Api-Check-errors-raised-by-mciissuese.patch (1,389 bytes) From 68cfe38428da79bdc9932cf998102a02db92c982 Fri, 11 Jan 2013 11:51:30 +0100 From: jeckyll <jeckyll@mantisbt.org> Date: Fri, 11 Jan 2013 10:56:42 +0100 Subject: [PATCH] fix #15356 [SOAP Api] Check errors raised by mci_issue_set_custom_field diff --git a/api/soap/mc_issue_api.php b/api/soap/mc_issue_api.php index f0c2fcb..e6c9b07 100644 --- a/api/soap/mc_issue_api.php +++ b/api/soap/mc_issue_api.php @@ -675,7 +675,9 @@ # submit the issue $t_issue_id = $t_bug_data->create(); - mci_issue_set_custom_fields( $t_issue_id, $p_issue['custom_fields'], false ); + $t_set_custom_field_error = mci_issue_set_custom_fields( $t_issue_id, $p_issue['custom_fields'], false ); + if ( $t_set_custom_field_error != null ) return $t_set_custom_field_error; + if ( isset ( $p_issue['monitors'] ) ) mci_issue_set_monitors( $t_issue_id , $t_user_id, $p_issue['monitors'] ); @@ -858,7 +860,9 @@ $t_bug_data->target_version = isset( $p_issue['target_version'] ) ? $p_issue['target_version'] : ''; } - mci_issue_set_custom_fields( $p_issue_id, $p_issue['custom_fields'], true ); + $t_set_custom_field_error = mci_issue_set_custom_fields( $p_issue_id, $p_issue['custom_fields'], true ); + if ( $t_set_custom_field_error != null ) return $t_set_custom_field_error; + if ( isset ( $p_issue['monitors'] ) ) mci_issue_set_monitors( $p_issue_id , $t_user_id, $p_issue['monitors'] ); fix-15356-SOAP-Api-Check-errors-raised-by-mciissuese.patch (1,389 bytes)

rombert 2013-01-09 05:15 reporter ~0034743	Submitting a patch is always a good idea, as it increases the chances of improvement eventually making it into MantisBT core. All contributions are welcome and greatly appreciated. Patch submissions can be made in several ways. In the order of preference: Send us a Pull Request on our Github repository [1] Attach a GIT patch to the issue Attach a Unified Diff, clearly specifying the patch's base release Kindly avoid to upload entire modified PHP files. Please make sure that your submissions adhere to our Coding Guidelines [2], if they don't your patch might be rejected. [1] https://github.com/mantisbt/mantisbt [2] http://www.mantisbt.org/wiki/doku.php/mantisbt:coding_guidelines

jeckyll 2013-01-09 18:01 reporter ~0034748	Interesting, i'm not very familar with the "Pull Request" thing but I will try it.

jeckyll 2013-01-10 13:41 reporter ~0034757	I realized that the "pull request" is github-specific, you cannot do that directly in your favorite git client (EGit for me). Anyway, here's the patch (in attachment). Hope it we be OK.

rombert 2013-01-10 16:41 reporter ~0034759	Can you please create a git patch rather than a diff? This allows me to apply it with git and makes sure the contribution is recorded as yours. With EGit you should use Team -> Commit and then fill in the commit message. Afterwards you can use Team -> Show in History to get to the history view, right click on your commit and then select Create Patch. Alternatively, if you're using the Mylyn Mantis connector from within Eclipse you can just drag and drop the commit from the history view on the task editor and the upload patch wizard will start.

jeckyll 2013-01-11 05:53 reporter ~0034769	Fix 0015356 - check the returned value of mci_issue_set_custom_fields in mc_issue_api.php Patch uploaded via mylyn-mantis connector

jeckyll 2013-01-11 05:57 reporter ~0034770 Last edited: 2013-01-11 06:07	Mylyn-Mantis did the trick. As I understand I could also use EGit, but I had to select the "email" format option in EGit "create patch" wizard.

rombert 2013-01-12 14:57 reporter ~0034785	Now fixed, thanks for the contribution!

grangeway 2013-04-05 17:56 reporter ~0036127	Marking as 'acknowledged' not resolved/closed to track that change gets ported to master-2.0.x branch

MantisBT: master 4d907078 2013-01-11 04:56 jeckyll Committer: rombert Details Diff	fix 0015356 [SOAP Api] Check errors raised by mci_issue_set_custom_field		Affected Issues 0015356
	mod - api/soap/mc_issue_api.php		Diff File
MantisBT: master-1.2.x 1e70c3cb 2013-01-11 04:56 jeckyll Committer: rombert Details Diff	fix 0015356 [SOAP Api] Check errors raised by mci_issue_set_custom_field		Affected Issues 0015356
	mod - api/soap/mc_issue_api.php		Diff File