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

Feat: Receive message attributes #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tothandras
Copy link
Contributor

I wonder if I should write a parser...
https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/SQS.html#receiveMessage-property

{
      MessageAttributes: {
       "City": {
         DataType: "String", 
         StringValue: "Any City"
        }, 
       "PostalCode": {
         DataType: "Number", 
         StringValue: "123"
        }
      }
}

would become

// message.attributes
{
  City: 'Any City',
  PostalCode: 123
}

@TomFrost
Copy link
Owner

TomFrost commented Feb 8, 2017

I would totally not hate this ;-)

@TomFrost
Copy link
Owner

TomFrost commented Feb 8, 2017

Are these attributes accessible in the resulting Message object? I haven't used them a lot in the past (generally just json-formatted message bodies) so I'm not super familiar with the structure.

@tothandras
Copy link
Contributor Author

Yes, they are on the "raw" message like this:

{ MessageId: '5d540919-3c3b-48be-8c8e-a185de588bb6',
  ReceiptHandle: 'AQE...',
  Body: '{"foo":"bar"}',
  MessageAttributes:
   { round:
      { StringValue: '0',
        StringListValues: [],
        BinaryListValues: [],
        DataType: 'Number' } } }

@tothandras
Copy link
Contributor Author

@TomFrost I've added attribute parsing, please take a look.

@tothandras
Copy link
Contributor Author

@TomFrost

@TomFrost
Copy link
Owner

Awesome! Two thoughts:

  1. Since a message can now be received in this format, it would be more intuitive to be able to turn around and send one in this format too. Could I bother you to write the opposite of this for sending? :). This would be a breaking change, but since this will be 1.0 anyway, now would be the time for it.

  2. I'm torn on the new dependency. If adding a dependency makes the most sense, I actually think I'd prefer including the entire lodash library instead of just the reduce function, with the thinking that many projects already include lodash and that would allow it to be reused, instead of forcing node to add even more code to RAM that is unlikely to ever be reused.

But, looking at the source for lodash.reduce, that's kind of an insane call stack for something we can do as simply as Object.keys(attributes).reduce(...). This is something we've been a bit sensitive to at TechnologyAdvice, after running into Semantic-Org/Semantic-UI-React#864 and seeing jdalton's response. For something as performance-sensitive as a high-volume poller, vanilla's probably the winner here.

@tothandras
Copy link
Contributor Author

@TomFrost sure, I'll do both. (just a side note: we decided to stay with RabbitMQ for now)

@regevbr
Copy link

regevbr commented Mar 10, 2019

@tothandras I have created an enhanced, typescriped library based on this one with your fix in it - https://github.com/PruvoNet/squiss-ts
Please be aware that the api is not fully backwards compatible. Please let me know if that helps you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants