Skip to content

Commit

Permalink
[ServiceBus] Remove verification of abort error messages in tests (#2…
Browse files Browse the repository at this point in the history
…9853)

After upgrading to `@azure/abort-controller` v2, some tests that verify
the
abort error messages have been failing. It turns out that in our async
operations because multiple callbacks are attached to the same abort
signal (for
example, retry logic, delay calls for timeout, authentication, auto
lock-renewing, etc.) and multiple abort errors are thrown reacting to
their
signal being aborted. After upgrading to `@azure/abort-controller` v2,
the order
of `AbortError`s being thrown have changed thus the error we catch in
the test
is now different from the one before upgrading. It is possible that our
previous
`AbortContoller` implementation of parent-children aborter pattern
implies
certain ordering which is different from when manually attaching
callbacks.
However, what's important to the consumer of our APIs in this scenario
is that
the operation is aborted, AbortErrors are thrown, and one of them is
caught,
even a different one than before.

This PR removes verification of the error messages.

I also realize that it is not useful to re-throw the error from delay
for
timeout operation, when the delay operation is aborted either because a
signal
is aborted, or because a operation it is waiting for succeeds. So that
error is
swallowed.

-------

### Packages impacted by this PR
`@Azure/service-bus`
  • Loading branch information
jeremymeng authored Jun 3, 2024
1 parent 6cb2dd4 commit 12a14b4
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
20 changes: 14 additions & 6 deletions sdk/servicebus/service-bus/src/core/managementClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,20 @@ export class ManagementClient extends LinkEntity<RequestResponseLink> {
if (!this.isOpen()) {
await Promise.race([
this._init(aborter.signal),
delay(retryTimeoutInMs, { abortSignal: aborter.signal }).then(() => {
throw {
name: "OperationTimeoutError",
message: "The management request timed out. Please try again later.",
};
}),
delay(retryTimeoutInMs, { abortSignal: aborter.signal }).then(
function onfulfilled() {
throw {
name: "OperationTimeoutError",
message:
"The initialization of management client timed out. Please try again later.",
};
},
function onrejected(_) {
managementClientLogger.verbose(
`The management client initialization has either completed or been cancelled.`,
);
},
),
]).finally(() => {
aborter.abort();
abortSignal?.removeEventListener("abort", abortListener);
Expand Down
6 changes: 0 additions & 6 deletions sdk/servicebus/service-bus/test/public/sessionsTests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import Long from "long";
const should = chai.should();
import chaiAsPromised from "chai-as-promised";
chai.use(chaiAsPromised);
import { StandardAbortMessage } from "@azure/core-amqp";
import {
ServiceBusReceivedMessage,
delay,
Expand All @@ -29,7 +28,6 @@ import sinon from "sinon";
import { ServiceBusSessionReceiverImpl } from "../../src/receivers/sessionReceiver";

let unexpectedError: Error | undefined;
const abortMsgRegex = new RegExp(StandardAbortMessage);

async function processError(args: ProcessErrorArgs): Promise<void> {
unexpectedError = args.error;
Expand Down Expand Up @@ -268,7 +266,6 @@ describe("session tests", () => {
await receiver.getSessionState({ abortSignal: controller.signal });
throw new Error(`Test failure`);
} catch (err: any) {
err.message.should.match(abortMsgRegex);
err.name.should.equal("AbortError");
}
});
Expand All @@ -281,7 +278,6 @@ describe("session tests", () => {
await receiver.setSessionState("why", { abortSignal: controller.signal });
throw new Error(`Test failure`);
} catch (err: any) {
err.message.should.match(abortMsgRegex);
err.name.should.equal("AbortError");
}
});
Expand All @@ -294,7 +290,6 @@ describe("session tests", () => {
await receiver.renewSessionLock({ abortSignal: controller.signal });
throw new Error(`Test failure`);
} catch (err: any) {
err.message.should.match(abortMsgRegex);
err.name.should.equal("AbortError");
}
});
Expand All @@ -307,7 +302,6 @@ describe("session tests", () => {
await receiver.receiveDeferredMessages([Long.ZERO], { abortSignal: controller.signal });
throw new Error(`Test failure`);
} catch (err: any) {
err.message.should.match(abortMsgRegex);
err.name.should.equal("AbortError");
}
});
Expand Down

0 comments on commit 12a14b4

Please sign in to comment.