Skip to content
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

Better handling of proxy errors / prevent infinite retries when exceptions occur #104

Merged
merged 1 commit into from
Feb 8, 2018

Conversation

HowardWolosky
Copy link
Member

An exception that the Proxy might return will not contiain a message property
within the exception's InnerMessage, but the current error reporting logic
assumed that there would be a message property and then tried to call Trim()
on it. This resulted in a You cannot call a method on a null-valued expression.
exception.

Because that exception occurred within a try/catch that was within a catch statement,
the outer catch ended up eating the exception, resulting in the retry loop continuing
to try the command, over and over again.

There are two distinct fixes in here as a result:

  1. Only try to call Trim() if InnerMessage.message exists, otherwise,
    just report back InnerMessage itself.

  2. Make sure that the outer catch statement ends with a throw to ensure
    that any inner exceptions thrown will propagate. As a result, we will
    also explicitly continue the loop if we are going to perform an auto-retry.

@@ -6,7 +6,7 @@
CompanyName = 'Microsoft Corporation'
Copyright = 'Copyright (C) Microsoft Corporation. All rights reserved.'

ModuleVersion = '1.14.2'
ModuleVersion = '1.14.3'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're already at 1.14.3!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We weren't at the time that I pushed the PR. Updated.

…exceptions occur

An exception that the Proxy might return will not contiain a `message` property
within the exception's `InnerMessage`, but the current error reporting logic
assumed that there would be a `message` property and then tried to call `Trim()`
on it.  This resulted in a `You cannot call a method on a null-valued expression.`
exception.

Because that exception occurred within a try/catch that was within a catch statement,
the outer catch ended up eating the exception, resulting in the retry loop continuing
to try the command, over and over again.

There are two distinct fixes in here as a result:

1. Only try to call `Trim()` if `InnerMessage.message` exists, otherwise,
   just report back `InnerMessage` itself.

2. Make sure that the outer catch statement ends with a `throw` to ensure
   that any inner exceptions thrown will propagate.  As a result, we will
   also explicitly `continue` the loop if we are going to perform an auto-retry.
@HowardWolosky HowardWolosky merged commit c62dc3e into microsoft:master Feb 8, 2018
@HowardWolosky HowardWolosky deleted the flightFix branch February 8, 2018 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants