Skip to content

Commit b26c8ec

Browse files
Francisco-GaminoFrancisco-Gamino
committed
Fixing odata filter for getting a webhook by runbook name; Fixing JobSchedule by setting the correct JobScheduleId; fixing exception handling for source control cmdlets; Adding logic to ExecuteCmdlet, so that when a ErrorResponseException is thrown, we check to see if ErrorResponseException.Body.Message is available. If not, we try to extracted from error message from ErrorResponseException.Response.Content, and we rethrow again using the extracted message. With this fix, the correct error message for invalid ResourceGroupName and AutomationAccountName is displayed to the user.
1 parent a4399b4 commit b26c8ec

10 files changed

+212
-211
lines changed

‎src/ResourceManager/Automation/Commands.Automation/Cmdlet/AzureAutomationBaseCmdlet.cs

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
using System.Runtime.Serialization.Json;
2626
using System.Text;
2727
using System.Xml.Linq;
28+
using Newtonsoft.Json;
29+
using Newtonsoft.Json.Linq;
2830

2931
namespace Microsoft.Azure.Commands.Automation.Cmdlet
3032
{
@@ -86,17 +88,52 @@ public override void ExecuteCmdlet()
8688
{
8789
if (string.IsNullOrEmpty(ErrorResponseException.Body.Code) && string.IsNullOrEmpty(ErrorResponseException.Body.Message))
8890
{
89-
string message = this.ParseErrorMessage(ErrorResponseException.Response.Content);
90-
if (!string.IsNullOrEmpty(message))
91+
if (!string.IsNullOrEmpty(ErrorResponseException.Response.Content))
9192
{
92-
throw new ErrorResponseException(message, ErrorResponseException);
93+
if (IsValidJsonObject(ErrorResponseException.Response.Content))
94+
{
95+
var message = ParseJson(ErrorResponseException.Response.Content);
96+
if (!string.IsNullOrEmpty(message))
97+
{
98+
throw new ErrorResponseException(message, ErrorResponseException);
99+
}
100+
}
93101
}
94102
}
95103

96104
throw new ErrorResponseException(ErrorResponseException.Body.Message, ErrorResponseException);
97105
}
98106
}
99107

108+
// This function parses two type of Json contents:
109+
// 1) "{\"error\":{\"code\":\"ResourceGroupNotFound\",\"message\":\"Resource group 'foobar' could not be found.\"}}"
110+
// 2) "{\"code\":\"ResourceGroupNotFound\",\"message\":\"Resource group 'foobar' could not be found.\"}"
111+
private string ParseJson(string value)
112+
{
113+
value = value.Trim();
114+
try
115+
{
116+
var nestedError = JsonConvert.DeserializeObject<AzureAutomationNestedErrorResponseMessage>(value);
117+
return nestedError.Error.Message;
118+
}
119+
catch
120+
{
121+
// Ignore the parsing error.
122+
}
123+
124+
try
125+
{
126+
var error = JsonConvert.DeserializeObject<AzureAutomationErrorResponseMessage>(value);
127+
return error.Message;
128+
}
129+
catch
130+
{
131+
// Ignore the parsing error.
132+
}
133+
134+
return null;
135+
}
136+
100137
protected bool GenerateCmdletOutput(object result)
101138
{
102139
var ret = true;
@@ -131,29 +168,24 @@ protected bool GenerateCmdletOutput(IEnumerable<object> results)
131168
return ret;
132169
}
133170

134-
private string ParseErrorMessage(string errorMessage)
171+
// This function determines if the given value is a Json object.
172+
private bool IsValidJsonObject(string value)
135173
{
136-
// The errorMessage is expected to be the error details in JSON format.
137-
// e.g. <string xmlns="http://schemas.microsoft.com/2003/10/Serialization/">{"code":"NotFound","message":"Certificate not found."}</string>
138-
try
174+
value = value.Trim();
175+
if (value.StartsWith("{") && value.EndsWith("}"))
139176
{
140-
using (var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(XDocument.Load(new StringReader(errorMessage)).Root.Value)))
177+
try
141178
{
142-
var serializer = new DataContractJsonSerializer(typeof(ErrorResponseException));
143-
var errorResponse = (ErrorResponseException)serializer.ReadObject(memoryStream);
144-
145-
if (!string.IsNullOrWhiteSpace(errorResponse.Message))
146-
{
147-
return errorResponse.Message;
148-
}
179+
var result = JObject.Parse(value);
180+
return true;
181+
}
182+
catch
183+
{
184+
// ignore the failure
185+
return false;
149186
}
150187
}
151-
catch (Exception)
152-
{
153-
// swallow the exception as we cannot parse the error message
154-
}
155-
156-
return null;
188+
return false;
157189
}
158190
}
159191
}

‎src/ResourceManager/Automation/Commands.Automation/Cmdlet/GetAzureAutomationSourceControlSyncJobOutput.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public class GetAzureAutomationSourceControlSyncJobOutput : AzureAutomationBaseC
3737
public string SourceControlName { get; set; }
3838

3939
/// <summary>
40-
/// Gets or sets the source contorl sync job id.
40+
/// Gets or sets the source control sync job id.
4141
/// </summary>
4242
[Parameter(Mandatory = true, ValueFromPipelineByPropertyName = true,
4343
HelpMessage = "The source control sync job id.")]

‎src/ResourceManager/Automation/Commands.Automation/Commands.Automation.csproj

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,9 @@
148148
<Compile Include="Common\AutomationPSClientSourceControl.cs" />
149149
<Compile Include="Common\AutomationPSClientWebhook.cs" />
150150
<Compile Include="Common\AutomationCmdletParameterSet.cs" />
151+
<Compile Include="Common\AzureAutomationNestedErrorResponseMessage.cs" />
151152
<Compile Include="Common\AzureAutomationOperationException.cs" />
153+
<Compile Include="Common\AzureAutomationErrorResponseMessage.cs" />
152154
<Compile Include="Common\Constants.cs" />
153155
<Compile Include="Common\DscNodeStatus.cs" />
154156
<Compile Include="Common\IAutomationPSClient.cs" />
@@ -263,7 +265,7 @@
263265
</Reference>
264266
</ItemGroup>
265267
<ItemGroup>
266-
</ItemGroup>
268+
</ItemGroup>
267269
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
268270
<Import Project="..\..\..\..\tools\Common.Dependencies.targets" />
269271
<Target Name="AfterBuild">

‎src/ResourceManager/Automation/Commands.Automation/Common/AutomationPSClientSourceControl.cs

Lines changed: 47 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -67,23 +67,6 @@ public Model.SourceControl CreateSourceControl(
6767
Requires.Argument("branch", branch).NotNullOrEmpty();
6868
}
6969

70-
bool sourceControlExists = true;
71-
72-
try
73-
{
74-
this.GetSourceControl(resourceGroupName, automationAccountName, name);
75-
}
76-
catch (ResourceNotFoundException)
77-
{
78-
sourceControlExists = false;
79-
}
80-
81-
if (sourceControlExists)
82-
{
83-
throw new AzureAutomationOperationException(
84-
string.Format(CultureInfo.CurrentCulture, Resources.SourceControlAlreadyExists, name));
85-
}
86-
8770
var decryptedAccessToken = Utils.GetStringFromSecureString(accessToken);
8871

8972
var createParams = new SourceControlCreateOrUpdateParameters(
@@ -102,7 +85,14 @@ public Model.SourceControl CreateSourceControl(
10285
name,
10386
createParams);
10487

105-
return new Model.SourceControl(sdkSourceControl, automationAccountName, resourceGroupName);
88+
Model.SourceControl result = null;
89+
90+
if (sdkSourceControl != null)
91+
{
92+
result = new Model.SourceControl(sdkSourceControl, automationAccountName, resourceGroupName);
93+
}
94+
95+
return result;
10696
}
10797

10898
public Model.SourceControl UpdateSourceControl(
@@ -119,17 +109,6 @@ public Model.SourceControl UpdateSourceControl(
119109
Requires.Argument("ResourceGroupName", resourceGroupName).NotNullOrEmpty();
120110
Requires.Argument("AutomationAccountName", automationAccountName).NotNullOrEmpty();
121111
Requires.Argument("name", name).NotNullOrEmpty();
122-
123-
Model.SourceControl existingSourceControl = null;
124-
try
125-
{
126-
existingSourceControl = this.GetSourceControl(resourceGroupName, automationAccountName, name);
127-
}
128-
catch (ResourceNotFoundException)
129-
{
130-
throw new AzureAutomationOperationException(
131-
string.Format(CultureInfo.CurrentCulture, Resources.SourceControlNotFound, name));
132-
}
133112

134113
var updateParams = new AutomationManagement.Models.SourceControlUpdateParameters();
135114

@@ -170,7 +149,14 @@ public Model.SourceControl UpdateSourceControl(
170149
name,
171150
updateParams);
172151

173-
return new Model.SourceControl(sdkSourceControl, automationAccountName, resourceGroupName);
152+
Model.SourceControl result = null;
153+
154+
if (sdkSourceControl != null)
155+
{
156+
result = new Model.SourceControl(sdkSourceControl, automationAccountName, resourceGroupName);
157+
}
158+
159+
return result;
174160
}
175161

176162
public void DeleteSourceControl(
@@ -182,20 +168,7 @@ public void DeleteSourceControl(
182168
Requires.Argument("automationAccountName", automationAccountName).NotNullOrEmpty();
183169
Requires.Argument("name", name).NotNullOrEmpty();
184170

185-
try
186-
{
187-
this.automationManagementClient.SourceControl.Delete(resourceGroupName, automationAccountName, name);
188-
}
189-
catch (CloudException cloudException)
190-
{
191-
if (cloudException.Response.StatusCode == System.Net.HttpStatusCode.NoContent)
192-
{
193-
throw new ResourceNotFoundException(typeof(SourceControl),
194-
string.Format(CultureInfo.CurrentCulture, Resources.SourceControlNotFound, name));
195-
}
196-
197-
throw;
198-
}
171+
this.automationManagementClient.SourceControl.Delete(resourceGroupName, automationAccountName, name);
199172
}
200173

201174
public Model.SourceControl GetSourceControl(
@@ -207,39 +180,16 @@ public Model.SourceControl GetSourceControl(
207180
Requires.Argument("automationAccountName", automationAccountName).NotNullOrEmpty();
208181
Requires.Argument("name", name).NotNullOrEmpty();
209182

210-
Model.SourceControl result = null;
211-
bool throwException = false;
212-
213-
try
214-
{
215-
var existingSourceControl = this.automationManagementClient.SourceControl.Get(
216-
resourceGroupName,
217-
automationAccountName,
218-
name);
183+
var existingSourceControl = this.automationManagementClient.SourceControl.Get(
184+
resourceGroupName,
185+
automationAccountName,
186+
name);
219187

220-
if (existingSourceControl != null)
221-
{
222-
result = new Model.SourceControl(existingSourceControl, automationAccountName, resourceGroupName);
223-
}
224-
else
225-
{
226-
throwException = true;
227-
}
228-
}
229-
catch (Exception ex)
230-
{
231-
if (ex is CloudException || ex is ErrorResponseException)
232-
{
233-
throwException = true;
234-
}
235-
else
236-
throw;
237-
}
188+
Model.SourceControl result = null;
238189

239-
if (throwException)
190+
if (existingSourceControl != null)
240191
{
241-
throw new ResourceNotFoundException(typeof(SourceControl),
242-
string.Format(CultureInfo.CurrentCulture, Resources.SourceControlNotFound, name));
192+
result = new Model.SourceControl(existingSourceControl, automationAccountName, resourceGroupName);
243193
}
244194

245195
return result;
@@ -283,30 +233,21 @@ public Model.SourceControlSyncJob StartSourceControlSyncJob(
283233
Requires.Argument("sourceControlName", sourceControlName).NotNullOrEmpty();
284234
Requires.Argument("syncJobId", syncJobId).NotNullOrEmpty();
285235

286-
bool syncJobExists = true;
287-
try
288-
{
289-
this.GetSourceControlSyncJob(resourceGroupName, automationAccountName, sourceControlName, syncJobId);
290-
}
291-
catch (ResourceNotFoundException)
292-
{
293-
syncJobExists = false;
294-
}
295-
296-
if (syncJobExists)
297-
{
298-
throw new AzureAutomationOperationException(
299-
string.Format(CultureInfo.CurrentCulture, Resources.SourceControlSyncJobAlreadyExist, syncJobId.ToString()));
300-
}
301-
302236
var sdkSyncJob = this.automationManagementClient.SourceControlSyncJob.Create(
303237
resourceGroupName,
304238
automationAccountName,
305239
sourceControlName,
306240
syncJobId,
307241
new SourceControlSyncJobCreateParameters(""));
308242

309-
return new Model.SourceControlSyncJob(resourceGroupName, automationAccountName, sourceControlName, sdkSyncJob);
243+
Model.SourceControlSyncJob result = null;
244+
245+
if (sdkSyncJob != null)
246+
{
247+
result = new Model.SourceControlSyncJob(resourceGroupName, automationAccountName, sourceControlName, sdkSyncJob);
248+
}
249+
250+
return result;
310251
}
311252

312253
public Model.SourceControlSyncJobRecord GetSourceControlSyncJob(
@@ -320,25 +261,17 @@ public Model.SourceControlSyncJobRecord GetSourceControlSyncJob(
320261
Requires.Argument("sourceControlName", sourceControlName).NotNullOrEmpty();
321262
Requires.Argument("syncJobId", syncJobId).NotNullOrEmpty();
322263

323-
Model.SourceControlSyncJobRecord result = null;
324-
325-
try
326-
{
327-
var existingSyncJob = this.automationManagementClient.SourceControlSyncJob.Get(
264+
var existingSyncJob = this.automationManagementClient.SourceControlSyncJob.Get(
328265
resourceGroupName,
329266
automationAccountName,
330267
sourceControlName,
331268
syncJobId);
332269

333-
if (existingSyncJob != null)
334-
{
335-
result = new Model.SourceControlSyncJobRecord(resourceGroupName, automationAccountName, sourceControlName, existingSyncJob);
336-
}
337-
}
338-
catch (ErrorResponseException)
270+
Model.SourceControlSyncJobRecord result = null;
271+
272+
if (existingSyncJob != null)
339273
{
340-
throw new ResourceNotFoundException(typeof(SourceControl),
341-
string.Format(CultureInfo.CurrentCulture, Resources.SourceControlSyncJobNotFound, syncJobId.ToString()));
274+
result = new Model.SourceControlSyncJobRecord(resourceGroupName, automationAccountName, sourceControlName, existingSyncJob);
342275
}
343276

344277
return result;
@@ -354,7 +287,6 @@ public Model.SourceControlSyncJobRecord GetSourceControlSyncJob(
354287
Requires.Argument("automationAccountName", automationAccountName).NotNullOrEmpty();
355288
Requires.Argument("sourceControlName", sourceControlName).NotNullOrEmpty();
356289

357-
// SourceControlListResponse comes from metadata.
358290
Rest.Azure.IPage<AutomationManagement.Models.SourceControlSyncJob> response;
359291

360292
if (string.IsNullOrEmpty(nextLink))
@@ -406,7 +338,7 @@ public Model.SourceControlSyncJobRecord GetSourceControlSyncJob(
406338

407339
nextLink = response.NextPageLink;
408340

409-
return response.Select(
341+
return response.Select(
410342
stream => new Model.SourceControlSyncJobStream(stream, resourceGroupName, automationAccountName, sourceControlName, syncJobId));
411343
}
412344

@@ -430,8 +362,15 @@ public SourceControlSyncJobStreamRecord GetSourceControlSyncJobStreamRecord(
430362
syncJobId,
431363
syncJobStreamId);
432364

433-
return new SourceControlSyncJobStreamRecord(
434-
response, resourceGroupName, automationAccountName, sourceControlName, syncJobId);
365+
SourceControlSyncJobStreamRecord result = null;
366+
367+
if (response != null)
368+
{
369+
result = new SourceControlSyncJobStreamRecord(
370+
response, resourceGroupName, automationAccountName, sourceControlName, syncJobId);
371+
}
372+
373+
return result;
435374
}
436375

437376
#region private helper functions

0 commit comments

Comments
 (0)