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

[service-bus] loosens parameter type in isServiceBusError type guard #12493

Merged
merged 2 commits into from
Nov 12, 2020

Conversation

chradek
Copy link
Contributor

@chradek chradek commented Nov 11, 2020

Fixes #12264

The goal with this change is to prevent exposing the AmqpError type to end-users. To accomplish this, the err parameter passed to isServiceBusError has been changed to be unknown. This doesn't change how the type guard can be used (while it is now more permissive, the end result should be the same.)

I did not add an entry to the changelog for this change since it doesn't change how isServiceBusError works, it just allows anything to be passed to it now without a compilation error. If this is a notable change I can add it to the changelog.

@chradek chradek added Client This issue points to a problem in the data-plane of the library. Service Bus labels Nov 11, 2020
@chradek chradek changed the title [service-bus] loosens paramter type in isServiceBusError type guard [service-bus] loosens parameter type in isServiceBusError type guard Nov 11, 2020
): err is ServiceBusError {
return (err as Error | ServiceBusError).name === "ServiceBusError";
export function isServiceBusError(err: unknown): err is ServiceBusError {
return (err as Error | ServiceBusError)?.name === "ServiceBusError";
Copy link
Contributor

Choose a reason for hiding this comment

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

can this not be just (err as Error) instead of (err as Error | ServiceBusError)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updates err: unknown to be err: any so I don't have to cast err at all in this line. For our purposes, any works fine.

@chradek chradek merged commit 62c1827 into Azure:master Nov 12, 2020
@chradek chradek deleted the sb-hide-amqp-error branch November 12, 2020 01:56
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-js that referenced this pull request Jan 20, 2021
[Fix broken S360 Issues][HDInsight] Fix S360 issues batch2 (Azure#12493)

* Add networkProperties and clusterId

* Add display.description and properties of operation.
Fix DeletionOperationResponse linting error
Add privateIPAddress in ConnectivityEndpoints and
ApplicationGetEndpoints

* Fix model validation error: update example

Co-authored-by: Zhenyu Zhou <zhezhou@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Service Bus] isServiceBusError should not expose AmqpError which is an internal type
2 participants