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] Bug Fix: Unexpected expiresAtUtc: Invalid Date in the receivedMessage #13543

Merged
15 commits merged into from Apr 14, 2021
Merged
3 changes: 2 additions & 1 deletion sdk/servicebus/service-bus/CHANGELOG.md
@@ -1,6 +1,6 @@
# Release History

## 7.1.0-beta.1 (2021-04-07)
## 7.1.0-beta.1 (Unreleased)

### New Features

Expand All @@ -11,6 +11,7 @@

### Bug fixes

- [Bug Fix] `expiresAtUtc` is `Invalid Date` in the received message when the ttl is not defined. Has been fixed in [#13543](https://github.com/Azure/azure-sdk-for-js/pull/13543)
- Some of the queue properties such as "forwardTo" and "autoDeleteOnIdle" were not being set as requested through the `ServiceBusAdministrationClient.createQueue` method because of a bug w.r.t the ordering of XML properties. The issue has been fixed in [#14692](https://github.com/Azure/azure-sdk-for-js/pull/14692).

## 7.0.4 (2021-03-31)
Expand Down
23 changes: 15 additions & 8 deletions sdk/servicebus/service-bus/src/serviceBusMessage.ts
Expand Up @@ -550,7 +550,12 @@ export function fromRheaMessage(
}
}

const props: any = {};
type PartialWritable<T> = Partial<
{
-readonly [P in keyof T]: T[P];
chradek marked this conversation as resolved.
Show resolved Hide resolved
}
>;
const props: PartialWritable<ServiceBusReceivedMessage> = {};
if (msg.message_annotations != null) {
if (msg.message_annotations[Constants.deadLetterSource] != null) {
props.deadLetterSource = msg.message_annotations[Constants.deadLetterSource];
Expand All @@ -572,18 +577,18 @@ export function fromRheaMessage(
props.lockedUntilUtc = new Date(msg.message_annotations[Constants.lockedUntil] as number);
}
}
if (msg.ttl != null && msg.ttl >= Constants.maxDurationValue - props.enqueuedTimeUtc.getTime()) {
props.expiresAtUtc = new Date(Constants.maxDurationValue);
} else {
props.expiresAtUtc = new Date(props.enqueuedTimeUtc.getTime() + msg.ttl!);
if (msg.ttl == null) msg.ttl = Constants.maxDurationValue;
if (props.enqueuedTimeUtc) {
Copy link
Member

Choose a reason for hiding this comment

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

Is props.enqueuedTime ever not set if a message has been received from an actual broker?

Copy link
Member Author

Choose a reason for hiding this comment

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

I hadn't seen such a case, but this gives the flexibility to set the props properly even if the service messes up.

props.expiresAtUtc = new Date(
Math.min(props.enqueuedTimeUtc.getTime() + msg.ttl, Constants.maxDurationValue)
);
}

const rawMessage = AmqpAnnotatedMessage.fromRheaMessage(msg);
rawMessage.bodyType = bodyType;

const rcvdsbmsg: ServiceBusReceivedMessage = {
_rawAmqpMessage: rawMessage,
_delivery: delivery,
chradek marked this conversation as resolved.
Show resolved Hide resolved
deliveryCount: msg.delivery_count,
lockToken:
delivery && delivery.tag && delivery.tag.length !== 0
Expand All @@ -599,8 +604,10 @@ export function fromRheaMessage(
: undefined,
...sbmsg,
...props,
deadLetterReason: sbmsg.applicationProperties?.DeadLetterReason,
deadLetterErrorDescription: sbmsg.applicationProperties?.DeadLetterErrorDescription
deadLetterReason: sbmsg.applicationProperties?.DeadLetterReason as string | undefined,
Copy link
Member

Choose a reason for hiding this comment

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

You are on a type-roll! ;)

deadLetterErrorDescription: sbmsg.applicationProperties?.DeadLetterErrorDescription as
| string
| undefined
};

logger.verbose("AmqpMessage to ServiceBusReceivedMessage: %O", rcvdsbmsg);
Expand Down
@@ -0,0 +1,41 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import chai from "chai";
import chaiAsPromised from "chai-as-promised";
import { ServiceBusMessage } from "../../../src";
import { TestMessage } from "../../public/utils/testUtils";
import { fromRheaMessage, toRheaMessage } from "../../../src/serviceBusMessage";
import { Message as RheaMessage } from "rhea-promise";
import { Constants } from "@azure/core-amqp";

const should = chai.should();
chai.use(chaiAsPromised);

describe("Message translations", () => {
describe("expiresAtUtc is not invalid on received message", function(): void {
async function verifyExpiresAtUtc(
transformMessage?: (msg: ServiceBusMessage) => ServiceBusMessage
): Promise<void> {
let testMessage = TestMessage.getSample();
if (transformMessage) testMessage = transformMessage(testMessage);

const rheaMsg: RheaMessage = {
...toRheaMessage(testMessage, { encode: (body) => body }),
message_annotations: { [Constants.enqueuedTime]: Date.now().valueOf() }
};
const expiresAtUtc = fromRheaMessage(rheaMsg).expiresAtUtc;
should.not.equal(expiresAtUtc?.toString(), "Invalid Date", "expiresAtUtc is Invalid Date");
}

it("ttl defined on sent message", async function(): Promise<void> {
await verifyExpiresAtUtc();
});

it("ttl undefined on sent message", async function(): Promise<void> {
await verifyExpiresAtUtc((msg: ServiceBusMessage) => {
return { ...msg, timeToLive: undefined };
});
});
});
});