Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.07k stars 1.19k forks source link

Scheduled messages return a Long sequence number but cancelScheduledMessages errors when these sequence numbers are passed as params #27368

Closed J35P1N closed 1 year ago

J35P1N commented 1 year ago

Describe the bug When using the Node SDK for Azure service bus, scheduling a message onto a queue using var response = await sender.scheduleMessages(message, req.body.deliveryTime);

is successful. This long value is then stored using MongoDb with the Typescript type of Long taken from the package "long": "^5.2.3" which I believe is the exact same package used in the service bus apk for nodeJs.

The issue arises when a message that has not yet met its schedule time needs to be removed from the queue. If I perform let deletion = await sender.cancelScheduledMessages(element);

where element is a type of either Long or Long[] as specified by the method parameters, the service bus always returns a 500 server error with the following message displayed within Swagger:

"The parameter \"sequenceNumbers\" should be of type \"Long\"" or "The parameter \"sequenceNumbers\" should be of type \"Long[]\""

As this is clearly passing up the correct Long object, could you please clarify why the cancelScheduledMessages method does not work as it should?

I have used the .Net SDK as that is my preferred development stack and I am able to cancel messages with the exact same process successfully.

If I am not supposed to be using an external library to support Long, then please point me in the direction of the documentation that specifies how this can be used in conjunction with the SDK as Node js does not natively support the Long type as it does not exist in a Javascript context.

Expected behavior Passing a Long value to cancelScheduledMessages should not produce a 500 and should delete the scheduled message from the queue

### Tasks
jeremymeng commented 1 year ago

Thanks for reporting this issue @J35P1N! I am taking a look now.

jeremymeng commented 1 year ago

@J35P1N I suspect that the format of the Long object was changed when doing saving to DB then reading from DB. Have you tried log the value to check whether they are the same before saving and after loading from DB? Are you using Longs api to serialize to and deserialize from, for example, bytes?

J35P1N commented 1 year ago

@jeremymeng The object is written to the MongoDB as Long using the library listed above and is read back out of the database successfully. Looking at the record using Mongo compass I can definitely see that the record has all the hallmarks of a Long object.

Screenshot 2023-10-11 at 09 06 52
J35P1N commented 1 year ago

For further clarification, I can see my message in the service bus using the peek option and we can see that It has a sequence number of 28.

Screenshot 2023-10-11 at 10 26 47

To eliminate the possibility of the long being saved as a different type, I have hard coded the following in my solution to convert the int 28 to its Long version in order to match the input expected for the sequenceNumber parameter.

Screenshot 2023-10-11 at 10 30 05

This still returns the following error:

Screenshot 2023-10-11 at 10 29 18

If I create a random Long[] and put the convert number to Long value inside and then pass it up to the cancel method:

Screenshot 2023-10-11 at 10 34 42

I get the following error instead:

Screenshot 2023-10-11 at 10 33 00

But if I peek the queue the entry is still there. Could it be that the schedule logic is not working correctly?

When creating the message and scheduling it onto the service bus I am passing in a date so that the message should be scheduled rather than immediately added to the queue so all should be working as expected

Screenshot 2023-10-11 at 10 41 14
jeremymeng commented 1 year ago

@J35P1N when you schedule a message, you specify a time that the message should appear in the queue. When you see it in the Service Bus Explorer that means it is already enters the queue thus cannot be cancelled. That's why you got the error of "Messages not found". I think instead of saving the Long object, you can do the following:

    const sequenceNumbers = await sender.scheduleMessages(messages, new Date( Date.now() + 30 * 60 * 1000));
    // convert to strings before saving to DB
    const sequenceNumberStrings = sequenceNumbers.map(n => n.toString());

Then when canceling:

    // convert strings back to Long after reading from DB
    const sequenceNumbers = sequenceNumberStrings.map(id => Long.fromString(id));
    await sender.cancelScheduledMessages(sequenceNumbers[0]);  // or sequenceNumbers. Either Long or Long[] should work
J35P1N commented 1 year ago

@jeremymeng Unfortunately that means the scheduleMessages method does not work. I've replaced my code with the code snippets you provided above but the message is still going straight onto the queue.

var response = await sender.scheduleMessages(message, new Date(req.body.deliveryTime + 30 * 60 * 1000));

Where req.body.deliveryTime is "2023-10-14T00:37:21.586Z"

Before the code change, that is the datetime I was passing in directly as the scheduledEnqueueTime parameter.

We want to future date the messages as we have an azure function that will then pick them up and deliver them where they need to go once they are placed on the queue. However, it would seem the scheduledEnqueueTime parameter is being ignored:

image

The code is more or less line for line exactly the same as I have done in C# before except it's using the Node Js methods and SDK instead of the .Net one.

jeremymeng commented 1 year ago

@J35P1N new Date("2023-10-14T00:37:21.586Z" + 30 * 60 * 1000) is an invalid date, but new Date( "2023-10-14T00:37:21.586Z") should work. I was able to schedule messages them cancel them without any errors.

    const sequenceNumbers = await sender.scheduleMessages(messages, new Date("2023-10-14T00:37:21.586Z"));

If possible, could you please set the DEBUG environment variable before running your code, and share the log with us (after sanitizing sensitive info). Depends on your OS, on Linux could be

env DEBUG=azure*,rhea* node app.js
jeremymeng commented 1 year ago

@J35P1N were you passing the string "2023-10-14T00:37:21.586Z" directly? The scheduleMessages method expects a Date object. I just tried and found that using the string, messages go straight to Active queue, while using new Date( "2023-10-14T00:37:21.586Z") messages go to the Scheduled queue. We probably should add a check in the SDK and throw error if the argument is not an instance of Date

J35P1N commented 1 year ago

@jeremymeng Sorry for the delay in reply. I've been sick with Covid the past couple of days and only just catching up. I can confirm that the combination of saving the Long sequence number into the database as a string and ensuring that the date is hard cast using the new Date() method has resolved the issue. Unfortunately, despire the date field being passed into the API that drives this logic being specifically type as date-time for some reason Typescript was not honouring this. I can see you have raised a tweak request which I think makes a lot of sense at the moment the scheduleMessage lets you get away with any object type being passed in but really it should only be a date. Really appreciate your help and the detailed responses, make it a lot easier for something who is unfortunately having to code in something that isn't their native language.

jeremymeng commented 1 year ago

@J35P1N Oh no. Hope you recover soon!

Glad to hear that your issue has been resolved. I had thought that maybe you were using JavaScript without the help of type checking. I suspect that either something is wrong in your TypeScript configuration since normally the type mismatch would be reported; or the string value is casted to Date somewhere then satisfied TypeScript in compile time.

I am going to close this issue now. Feel free to reach out if you have any new questions or problems.