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

[Event Hubs] Use AwaitableSender in lieu of Sender #4446

Merged
merged 25 commits into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
260b0e6
[Event Hubs] Introduce timeoutInMs on RetryOptions (#4239)
ramya0820 Jul 11, 2019
c74f5e4
Merge branch 'master' of https://github.com/ramya0820/azure-sdk-for-js
Jul 13, 2019
00016cb
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
ramya0820 Jul 15, 2019
5c12c13
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
ramya0820 Jul 16, 2019
2081256
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
Jul 17, 2019
f3511e0
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
ramya0820 Jul 19, 2019
7f00436
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
ramya0820 Jul 20, 2019
e2dc78b
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
Jul 23, 2019
232390c
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js
Jul 23, 2019
05046df
Replace Sender -> AwaitableSender
Jul 26, 2019
c58e6eb
Address comments
Jul 26, 2019
298990b
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…
Jul 26, 2019
5956ffa
Remove comment
Jul 30, 2019
b2633be
Pass sendTimeout to AwaitableSender
Jul 30, 2019
264794c
Refactor translate()
Jul 30, 2019
aa664ff
Prepend rejects with return
Jul 31, 2019
25e3f63
Refactor translate()
Jul 31, 2019
edf1949
Add tests
Jul 31, 2019
ebd7dde
Refactor and fix timeout to init()
Jul 31, 2019
11dbe10
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…
Jul 31, 2019
cb14ba3
Address comments
Aug 1, 2019
f5f0428
Address comments
Aug 1, 2019
abc9689
Refactor translate()
Aug 2, 2019
ffca3c9
Address comments
Aug 2, 2019
dee862c
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…
Aug 2, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 36 additions & 27 deletions sdk/core/core-amqp/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -481,12 +481,20 @@ export const retryableErrors: string[] = [
"ServerBusyError",
"ServiceUnavailableError",
"OperationCancelledError",

// OperationTimeoutError occurs when the service fails to respond within a given timeframe.
// Since reasons for such failures can be transient, this is treated as a retryable error.
"OperationTimeoutError",

"SenderBusyError",
"MessagingError",
"DetachForcedError",
"ConnectionForcedError",
"TransferLimitExceededError"
"TransferLimitExceededError",

// InsufficientCreditError occurs when the number of credits available on Rhea link is insufficient.
// Since reasons for such shortage can be transient such as for pending delivery of messages, this is treated as a retryable error.
"InsufficientCreditError"
];

/**
Expand Down Expand Up @@ -553,28 +561,9 @@ export function translate(err: AmqpError | Error): MessagingError {

let error: MessagingError = err as MessagingError;

// OperationTimeoutError occurs when the service fails to respond within a given timeframe.
// Since reasons for such failures can be transient, this is treated as a retryable error.
if (
// instanceof checks on custom Errors doesn't work without manually setting the prototype within the error.
// Must do a name check until OperationTimeoutError is updated, and that doesn't break compatibility
// https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work
(err as Error).name === "OperationTimeoutError"
) {
error.retryable = true;
return error;
}

// Built-in errors like TypeError and RangeError should not be retryable as these indicate issues
// with user input and not an issue with the Messaging process.
if (
err instanceof TypeError ||
err instanceof RangeError ||
// instanceof checks on custom Errors doesn't work without manually setting the prototype within the error.
// Must do a name check until AbortError is updated, and that doesn't break compatibility
// https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work
(err as Error).name === "AbortError"
) {
if (err instanceof TypeError || err instanceof RangeError) {
error.retryable = false;
return error;
}
Expand Down Expand Up @@ -602,7 +591,10 @@ export function translate(err: AmqpError | Error): MessagingError {
// not found
error.retryable = false;
}
} else if (isSystemError(err)) {
return error;
}

if (isSystemError(err)) {
// translate
const condition = (err as any).code;
const description = (err as Error).message;
Expand All @@ -617,15 +609,32 @@ export function translate(err: AmqpError | Error): MessagingError {
// not found
error.retryable = false;
}
} else if (isBrowserWebsocketError(err)) {
return error;
}

if (isBrowserWebsocketError(err)) {
// Translate browser communication errors during opening handshake to generic SeviceCommunicationError
error = new MessagingError("Websocket connection failed.");
error.name = ConditionErrorNameMapper[ErrorNameConditionMapper.ServiceCommunicationError];
error.retryable = false;
} else {
// Translate a generic error into MessagingError.
error = new MessagingError((err as Error).message);
error.stack = (err as Error).stack;
return error;
}

// instanceof checks on custom Errors doesn't work without manually setting the prototype within the error.
// Must do a name check until the custom error is updated, and that doesn't break compatibility
// https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work
const errorName = (err as Error).name;
if (retryableErrors.indexOf(errorName) > -1) {
error.retryable = true;
return error;
}
if (errorName === "AbortError") {
error.retryable = false;
return error;
}

// Translate a generic error into MessagingError.
error = new MessagingError((err as Error).message);
error.stack = (err as Error).stack;
return error;
}
46 changes: 34 additions & 12 deletions sdk/core/core-amqp/test/errors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as chai from "chai";
const should = chai.should();

import * as Errors from "../src/errors";
import { AbortError } from "@azure/abort-controller";

class AMQPError {
name = "AmqpProtocolError";
Expand All @@ -30,6 +31,35 @@ describe("Errors", function() {
translatedError.message.should.equal(ehError.message);
});

it("Sets retryable to true, if input is custom error and name is OperationTimeoutError", function() {
const err = new Error("error message");
err.name = "OperationTimeoutError";
const translatedError = Errors.translate(err);
should.equal(translatedError.name === "OperationTimeoutError", true);
translatedError.message.should.equal(err.message);
translatedError.stack!.should.equal(err.stack);
translatedError.retryable.should.equal(true);
});

it("Sets retryable to true, if input is custom error and name is InsufficientCreditError", function() {
const err = new Error("error message");
err.name = "InsufficientCreditError";
const translatedError = Errors.translate(err);
should.equal(translatedError.name === "InsufficientCreditError", true);
translatedError.message.should.equal(err.message);
translatedError.stack!.should.equal(err.stack);
translatedError.retryable.should.equal(true);
});

it("Sets retryable to false, if input is the custom AbortError", function() {
const err = new AbortError("error message");
const translatedError = Errors.translate(err);
should.equal(translatedError.name === "AbortError", true);
translatedError.message.should.equal(err.message);
translatedError.stack!.should.equal(err.stack);
translatedError.retryable.should.equal(false);
});

it("Sets retryable to false, and acts as a passthrough if the input is TypeError", function() {
const err = new TypeError("This is a wrong type!!");
const translatedError = Errors.translate(err);
Expand Down Expand Up @@ -67,10 +97,7 @@ describe("Errors", function() {
{ from: "<unknown>", to: "MessagingError" }
].forEach(function(mapping) {
it("translates " + mapping.from + " into " + mapping.to, function() {
const err: any = new AMQPError(
mapping.from as any,
mapping.message as any
);
const err: any = new AMQPError(mapping.from as any, mapping.message as any);
const translatedError = <Errors.MessagingError>Errors.translate(err);
translatedError.name.should.equal(mapping.to);
if (
Expand Down Expand Up @@ -113,18 +140,13 @@ describe("Errors", function() {
code: "ESOMETHINGRANDOM",
errno: "ESOMETHINGRANDOM",
syscall: "read",
message:
"code: ESOMETHINGRANDOM, errno: ESOMETHINGRANDOM, syscall: read"
message: "code: ESOMETHINGRANDOM, errno: ESOMETHINGRANDOM, syscall: read"
}
].forEach(function(mapping) {
it(
"SystemError from node.js with code: '" +
mapping.code +
"' to a MessagingError",
"SystemError from node.js with code: '" + mapping.code + "' to a MessagingError",
function() {
const translatedError = <Errors.MessagingError>(
Errors.translate(mapping as any)
);
const translatedError = <Errors.MessagingError>Errors.translate(mapping as any);
if (mapping.code === "ECONNRESET") {
translatedError.name.should.equal("ServiceUnavailableError");
translatedError.retryable.should.equal(true);
Expand Down
4 changes: 3 additions & 1 deletion sdk/eventhub/event-hubs/review/event-hubs.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { AbortSignalLike } from '@azure/abort-controller';
import { AmqpError } from 'rhea-promise';
import { AwaitableSender } from 'rhea-promise';
import { ConnectionContextBase } from '@azure/core-amqp';
import { DataTransformer } from '@azure/core-amqp';
import { DefaultDataTransformer } from '@azure/core-amqp';
Expand All @@ -16,7 +17,6 @@ import { MessagingError } from '@azure/core-amqp';
import { Receiver } from 'rhea-promise';
import { ReceiverOptions } from 'rhea-promise';
import { RetryOptions } from '@azure/core-amqp';
import { Sender } from 'rhea-promise';
import { SharedKeyCredential } from '@azure/core-amqp';
import { TokenCredential } from '@azure/core-amqp';
import { TokenType } from '@azure/core-amqp';
Expand Down Expand Up @@ -279,6 +279,8 @@ export class ReceiveHandler {
stop(): Promise<void>;
}

export { RetryOptions }

// @public
export interface SendOptions {
abortSignal?: AbortSignalLike;
Expand Down
Loading