[Service Bus] Bug Fix: Unexpected `expiresAtUtc: Invalid Date` in the receivedMessage #13543

HarshaNalluru · 2021-02-02T22:36:39Z

Bug

"Invalid date" for the expiresAtUtc property

azure:service-bus:messages:verbose AmqpMessage to ServiceBusReceivedMessage: {
  _amqpAnnotatedMessage: {
    header: { deliveryCount: 0 },
  .
  .
  .
  expiresAtUtc: Invalid Date,
  .
  .
  .
}

Background

"ttl" on the received message will be undefined if the sent message doesn't have the "ttl" property set to a value.
In this case, the service assumes the default infinite value for expiry time.
Reference: https://docs.microsoft.com/en-us/azure/service-bus-messaging/message-expiration#entity-level-expiration

Cause (expiresAtUtc)

In the SDK, for a received message, we calculate the expiresAtUtc using the "ttl" and the enqueued time.

If the msg.ttl > max_duration - msg.enqueuedTime, calculate msg.expiresAtUtc with max_duration
If the msg.ttl < max_duration - msg.enqueuedTime, calculate msg.expiresAtUtc with msg.enqueuedTime + msg.ttl
If the msg.ttl is not defined on the received message, we return Invalid Date for the expiresAtUtc property.

.NET SDK

TTL is defaulted to TimeSpan.MaxValue in .NET.
For ExpiresAt calculation:

if AbsoluteExpiryTime is populated we use that.
if TTL > maxDateTime - EnqueuedTime, use TimeSpan.MaxValue
Else use EnqueuedTime + TTL

Fix for JS SDK

If the msg.ttl is not defined, set it to the max value as the service says that. This allows the proper calculation of expiresAtUtc.

TODO

Bug Fix
Test
Changelog

…to harshan/sb/issue/12883

check-enforcer · 2021-02-02T22:42:00Z

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run js - [service] - ci

chradek

Overall strategy looks good, but had some questions.

sdk/servicebus/service-bus/src/serviceBusMessage.ts

chradek · 2021-02-03T23:48:43Z

sdk/servicebus/service-bus/src/serviceBusMessage.ts

+    deadLetterReason: sbmsg.applicationProperties?.DeadLetterReason as string,
+    deadLetterErrorDescription: sbmsg.applicationProperties?.DeadLetterErrorDescription as string


Why do you need to add as string? Couldn't these be undefined?

Good point, undefined is allowed.

…to harshan/sb/issue/12883

richardpark-msft · 2021-04-14T20:59:55Z

sdk/servicebus/service-bus/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Release History

+## 7.1.0-beta.2 (2021-04-07)
+
+- [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)


Just a note, I have another PR where I'm just removing the date for .beta.1 since we never released it. So your changes would actually go there.

Oops, forgot!

richardpark-msft · 2021-04-14T21:04:26Z

sdk/servicebus/service-bus/src/serviceBusMessage.ts

-  } else {
-    props.expiresAtUtc = new Date(props.enqueuedTimeUtc.getTime() + msg.ttl!);
+  if (msg.ttl == null) msg.ttl = Constants.maxDurationValue;
+  if (props.enqueuedTimeUtc) {


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

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

richardpark-msft · 2021-04-14T21:08:23Z

sdk/servicebus/service-bus/src/serviceBusMessage.ts

-    props.expiresAtUtc = new Date(props.enqueuedTimeUtc.getTime() + msg.ttl!);
+  if (msg.ttl == null) msg.ttl = Constants.maxDurationValue;
+  if (props.enqueuedTimeUtc) {
+    if (msg.ttl >= Constants.maxDurationValue - props.enqueuedTimeUtc.getTime()) {


(is this the same?)
props.expiresAtUtc = Math.min(props.enqueuedTimeUtc.getTime() + msg.ttl, Constants.maxDurationValue)

Great catch

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

richardpark-msft · 2021-04-14T21:09:43Z

sdk/servicebus/service-bus/src/serviceBusMessage.ts

@@ -599,8 +606,10 @@ export function fromRheaMessage(
        : undefined,
    ...sbmsg,
    ...props,
-    deadLetterReason: sbmsg.applicationProperties?.DeadLetterReason,
-    deadLetterErrorDescription: sbmsg.applicationProperties?.DeadLetterErrorDescription
+    deadLetterReason: sbmsg.applicationProperties?.DeadLetterReason as string | undefined,


You are on a type-roll! ;)

richardpark-msft · 2021-04-14T21:10:11Z

sdk/servicebus/service-bus/test/public/receivedMessageProps.spec.ts

@@ -0,0 +1,41 @@
+// Copyright (c) Microsoft Corporation.


These are perfect unit tests - can you move this into internal/unit?

richardpark-msft

Looks good, glad you were able to remove yet one more spot where we didn't have typings :)

sdk/servicebus/service-bus/src/serviceBusMessage.ts

ghost · 2021-04-14T23:33:26Z

Hello @HarshaNalluru!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

… receivedMessage (Azure#13543) Fixes Azure#12883 ## Bug "Invalid date" for the `expiresAtUtc` property ``` azure:service-bus:messages:verbose AmqpMessage to ServiceBusReceivedMessage: { _amqpAnnotatedMessage: { header: { deliveryCount: 0 }, . . . expiresAtUtc: Invalid Date, . . . } ``` ## Background "ttl" on the received message will be undefined if the sent message doesn't have the "ttl" property set to a value. In this case, the service assumes the default infinite value for expiry time. Reference: https://docs.microsoft.com/en-us/azure/service-bus-messaging/message-expiration#entity-level-expiration ### Cause (expiresAtUtc) In the SDK, for a received message, we calculate the expiresAtUtc using the "ttl" and the enqueued time. - If the msg.ttl > max_duration - msg.enqueuedTime, calculate msg.expiresAtUtc with max_duration - If the msg.ttl < max_duration - msg.enqueuedTime, calculate msg.expiresAtUtc with msg.enqueuedTime + msg.ttl - If the msg.ttl is not defined on the received message, we return Invalid Date for the expiresAtUtc property. ## .NET SDK TTL is defaulted to TimeSpan.MaxValue in .NET. For ExpiresAt calculation: - if AbsoluteExpiryTime is populated we use that. - if TTL > maxDateTime - EnqueuedTime, use TimeSpan.MaxValue - Else use EnqueuedTime + TTL ## Fix for JS SDK If the msg.ttl is not defined, set it to the max value as the service says that. This allows the proper calculation of `expiresAtUtc`. ## TODO - [x] Bug Fix - [x] Test - [x] Changelog

HarshaNalluru added 5 commits January 15, 2021 16:42

checkpoint

cc9e0c0

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

de38cb1

…to harshan/sb/issue/12883

fix expiresAtUtc calculation

01606e1

package.json

b5d968f

if (props.enqueuedTimeUtc)

a8a7de4

ghost added the Service Bus label Feb 2, 2021

HarshaNalluru changed the title ~~[Service Bus]~~ [Service Bus] Bug Fix: Unexpected expiresAtUtc: Invalid Date in the receivedMessage Feb 2, 2021

chradek reviewed Feb 3, 2021

View reviewed changes

HarshaNalluru added 7 commits March 30, 2021 02:04

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

09e3042

…to harshan/sb/issue/12883

Merge branch 'master' of https://github.com/Azure/azure-sdk-for-js in…

adee2ad

…to harshan/sb/issue/12883

compile error

b46b874

string | undefined

9b8507e

undo unrelated changes in tests

9abf78f

expiresAtUtc unit tests

74e6806

changelog

d8980ed

HarshaNalluru marked this pull request as ready for review April 14, 2021 09:37

HarshaNalluru requested review from ramya-rao-a and richardpark-msft as code owners April 14, 2021 09:37

HarshaNalluru requested a review from chradek April 14, 2021 09:37

richardpark-msft reviewed Apr 14, 2021

View reviewed changes

moved to unreleased

cf262c2

richardpark-msft reviewed Apr 14, 2021

View reviewed changes

richardpark-msft approved these changes Apr 14, 2021

View reviewed changes

HarshaNalluru added 2 commits April 14, 2021 14:12

move to unit test folder

cf7be25

Math.min

35118da

chradek reviewed Apr 14, 2021

View reviewed changes

sdk/servicebus/service-bus/src/serviceBusMessage.ts Show resolved Hide resolved

sdk/servicebus/service-bus/src/serviceBusMessage.ts Show resolved Hide resolved

chradek approved these changes Apr 14, 2021

View reviewed changes

HarshaNalluru added the auto-merge label Apr 14, 2021

ghost merged commit 172df4f into Azure:master Apr 14, 2021

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Service Bus] Bug Fix: Unexpected `expiresAtUtc: Invalid Date` in the receivedMessage #13543

[Service Bus] Bug Fix: Unexpected `expiresAtUtc: Invalid Date` in the receivedMessage #13543

HarshaNalluru commented Feb 2, 2021 •

edited

check-enforcer bot commented Feb 2, 2021

chradek left a comment

chradek Feb 3, 2021

HarshaNalluru Apr 14, 2021

richardpark-msft Apr 14, 2021

HarshaNalluru Apr 14, 2021

richardpark-msft Apr 14, 2021

HarshaNalluru Apr 14, 2021

richardpark-msft Apr 14, 2021

HarshaNalluru Apr 14, 2021

HarshaNalluru Apr 14, 2021 •

edited

richardpark-msft Apr 14, 2021

richardpark-msft Apr 14, 2021

richardpark-msft left a comment

ghost commented Apr 14, 2021

		deadLetterReason: sbmsg.applicationProperties?.DeadLetterReason as string,
		deadLetterErrorDescription: sbmsg.applicationProperties?.DeadLetterErrorDescription as string

[Service Bus] Bug Fix: Unexpected expiresAtUtc: Invalid Date in the receivedMessage #13543

[Service Bus] Bug Fix: Unexpected expiresAtUtc: Invalid Date in the receivedMessage #13543

Conversation

HarshaNalluru commented Feb 2, 2021 • edited

Bug

Background

Cause (expiresAtUtc)

.NET SDK

Fix for JS SDK

TODO

check-enforcer bot commented Feb 2, 2021

What is Check Enforcer?

Why am I getting this message?

What should I do now?

What if I am onboarding a new service?

chradek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HarshaNalluru Apr 14, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richardpark-msft left a comment

Choose a reason for hiding this comment

ghost commented Apr 14, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[Service Bus] Bug Fix: Unexpected `expiresAtUtc: Invalid Date` in the receivedMessage #13543

[Service Bus] Bug Fix: Unexpected `expiresAtUtc: Invalid Date` in the receivedMessage #13543

HarshaNalluru commented Feb 2, 2021 •

edited

HarshaNalluru Apr 14, 2021 •

edited

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.