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

Error sending scheduled Message, null payload. #6816

Closed
angelqmx opened this issue Jan 7, 2020 · 12 comments
Closed

Error sending scheduled Message, null payload. #6816

angelqmx opened this issue Jan 7, 2020 · 12 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Bus
Milestone

Comments

@angelqmx
Copy link

angelqmx commented Jan 7, 2020

When trying to send a schedule message using the code

let message={
        body:  msg,
        label: "Scientist"
 }
 await sender.scheduleMessage(d,message);

The message is delivered on th correct time but the body is null. After checking the code on the sdk, on the schedule message the body is not being encrypted (which is diferent when we do a normal send), so if i add the line:

let message={
        body:  sender._context.namespace.dataTransformer.encode(msg),
        label: "Scientist"
 }
 await sender.scheduleMessage(d,message);

The message is delivered with the correct body.

@ramya-rao-a
Copy link
Contributor

Thanks for reporting @angelqmx

Looks like the spec for scheduling messages also point to the need for the message to be encoded.

@HarshaNalluru Can you please take a look at this?

@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Service Bus labels Jan 8, 2020
@triage-new-issues triage-new-issues bot removed the triage label Jan 8, 2020
@ramya-rao-a ramya-rao-a added this to the [2020] February milestone Jan 8, 2020
@ramya-rao-a ramya-rao-a added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Jan 8, 2020
@HarshaNalluru
Copy link
Member

Hey @angelqmx,

Thanks for reporting.

  • I couldn't repro the part of issue where you said the body is null.
    Can you let me know how you're receiving the message that you had sent?

  • Though I couldn't repro the body being null part, I could compare and observe a difference between the sent messages from scheduleMessage() and send() methods in the Service Bus Explorer.
    For instance, when the message.body = "hello world" for the sent message,

    • image
    • Though the Service Bus Explorer shows some difference, there is no difference in the received message(through the Receiver.registerMessageHandler)
    • Similarly for the messages that are not just strings,
      Example - with body as {key: "hello world"} or ["this","that"]
    • I'm inclined to resolve this issue by encoding the message in the SDK to make it similar to the send() method.
  • Can you provide a little more info on how you got the message.body as null so that I can repro on my end properly?

    • What is the message that you've sent and how you're receiving it? Even better if you can provide sample code to repro.

@angelqmx
Copy link
Author

angelqmx commented Jan 9, 2020

hello @HarshaNalluru today i have i very busy day i will try to make the example tomorrow or the weekend, in particular i was using a azure function to consume the message maybe is part of the bug. Gretings

@HarshaNalluru
Copy link
Member

Thanks, @angelqmx !!

@angelqmx
Copy link
Author

@HarshaNalluru here is the example
The code without endcoding
`
var argv = process.argv.slice(2);
var dotenv;

  if(argv[0]){
    console.log(argv[0]);
      dotenv = require('dotenv').config({path:"./env."+argv[0]});
    if (dotenv.error) {
        console.error("You haven't the file .env, please create the file .env with the content of the file dev.stage or dev.prod depending of the environment you need!!");
        process.exit(1);
    }
  }else{
    var argv = process.argv.slice(2);
    console.error("You haven't the file env argument, please use stage, prod  or dev depending of the environment you need!!");
  }





  const SCONNECTION = process.env.SERVICEBUS_HOST;
  const { ServiceBusClient } = require("@azure/service-bus"); 

  // Define connection string and related Service Bus entity names here
  const topicName = "testtopic"; 


  const sbClient = ServiceBusClient.createFromConnectionString(SCONNECTION); 
  const topicClient = sbClient.createTopicClient(topicName);
  const sender = topicClient.createSender();

    


  let sendTopic = async function(){
      let d = new Date();
        d.setSeconds( d.getSeconds() + 30 );

      message={
          body:  "Hello world"
        };
      await sender.scheduleMessage(d,message);
  }


  async function main(){
   

    try {
      
       
     
       
        
      await sendTopic();

      await topicClient.close();
    } finally {
      await sbClient.close();
    }
  }

  main().catch((err) => {
    console.log("Error occurred: ", err);
  });

`
And the result in the functions
NoEncr

Using the encoding:
`
var argv = process.argv.slice(2);
var dotenv;

  if(argv[0]){
    console.log(argv[0]);
      dotenv = require('dotenv').config({path:"./env."+argv[0]});
    if (dotenv.error) {
        console.error("You haven't the file .env, please create the file .env with the content of the file dev.stage or dev.prod depending of the environment you need!!");
        process.exit(1);
    }
  }else{
    var argv = process.argv.slice(2);
    console.error("You haven't the file env argument, please use stage, prod  or dev depending of the environment you need!!");
  }





  const SCONNECTION = process.env.SERVICEBUS_HOST;
  const { ServiceBusClient } = require("@azure/service-bus"); 

  // Define connection string and related Service Bus entity names here
  const topicName = "testtopic"; 


  const sbClient = ServiceBusClient.createFromConnectionString(SCONNECTION); 
  const topicClient = sbClient.createTopicClient(topicName);
  const sender = topicClient.createSender();

    


  let sendTopic = async function(){
      let d = new Date();
        d.setSeconds( d.getSeconds() + 30 );

      message={
          body:   sender._context.namespace.dataTransformer.encode("Hello world")
        };
      await sender.scheduleMessage(d,message);
  }


  async function main(){
   

    try {
      
       
     
       
        
      await sendTopic();

      await topicClient.close();
    } finally {
      await sbClient.close();
    }
  }

  main().catch((err) => {
    console.log("Error occurred: ", err);
  });

`
And th ersult in the function:
Encr

As we can see the firts image show undifined, while the second image show the correct message

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 13, 2020

Thanks for the detailed repro and your investigation, @angelqmx.
I could verify by consuming the message with the azure functions, message received is undefined when sent through the current scheduleMessage.

Encoding the message seems to fix the issues here

  • undefined in the Azure Functions,
  • a non-null corrupted message in the Service Bus Explorer
    (Probably, both are caused by the same underlying issue.)

Here is a branch with the change that fixes the issue - HarshaNalluru@36ab852
With this change, message.body is not undefined anymore when consumed with azure functions.

@angelqmx, Since you have found the fix, are you interested in contributing to the repo by making a PR to fix the issue?
To-Do - more or less, this - HarshaNalluru@36ab852

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 13, 2020

In retrospection, the issue seems to be that Azure Functions/Service Bus Explorer might be employing different ways to consume the message(probably leveraging the .NET SDK or some other technique) which is why we couldn't find the bug any sooner since our tests are completely relying on and meant to test the JS SDK only.
Any thoughts on cross-language testing, @ramya-rao-a ?

@angelqmx
Copy link
Author

I have no problem doing the PR, just one question after the fix encriptyng the message before sending is going to stop working? if that is the case if i update the lib is going to start failing?

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 14, 2020

after the fix, encrypting the message before sending is going to stop working?

Yes, but only after we release a new version of @azure/service-bus with the fix and you update to that new version. I'm inclining to think that this is a non-breaking change and we will probably release a patch version to fix this issue. (Unless @ramya-rao-a thinks otherwise.)

Update - updated after the discussion with @ramya-rao-a, see below - #6816 (comment)

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Jan 15, 2020

Some more info, @angelqmx!!

[ Discussed offline with @ramya-rao-a ] : Summary

Bug

  • Send a message with scheduleMessage() method from the SDK.
  • Receive the message through the Azure Function Triggers or Service Bus Explorer, the received message is undefined or corrupted.
    (the same is not the case when received the message with Receiver.registerMessageHandler from the sdk, which is the reason why the bug wasn't discovered till now)

Fix

  • Encoding the message would fix the above issues.
  • Encoding is supposed to be done in the SDK with DataTransformer.encode() method in the scheduleMessage() function.
  • HarshaNalluru@36ab852

Since fixing the bug(encoding in the library) would break existing users who are already doing the encoding on their end, we consider this to be a breaking change and will be shipping the fix in the next major version update.

Till then, we are suggesting the following workaround.

Workaround
While your workaround works perfectly fine, Typescript users cannot make use of it as the _context is a private property on the sender. Therefore, we suggest the below workaround:

  • Import DefaultDataTransformer from "@azure/amqp-common" library.
    In typescript, import { DefaultDataTransformer } from "@azure/amqp-common";
    In javascript, const { DefaultDataTransformer } = require("@azure/amqp-common");
  • Update the message body before calling the scheduleMessage() method to send the message as follows
    • Instantiate the data transformer used by the sdk:
      const dt = new DefaultDataTransformer();
    • When you need to schedule the message, encode the message body before sending:
      message.body = dt.encode(message.body);

Users are requested to leverage the workaround and we will be shipping the fix only in the next major release for this issue.

@HarshaNalluru
Copy link
Member

#7372, fix will be shipped starting from the version-2.0.0-preview.1.

@xirzec xirzec modified the milestones: [2020] February, [2020] April Mar 9, 2020
@ramya-rao-a
Copy link
Contributor

Closing this issue as the workaround for when using v1 of the @azure/service-bus package is listed above #6816 (comment)

The next major version of the package will not need the workaround.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Bus
Projects
None yet
Development

No branches or pull requests

4 participants