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

Empty sns subject not allowed since >= 4.13.0 #304

Open
vsmart opened this issue Mar 7, 2019 · 0 comments
Open

Empty sns subject not allowed since >= 4.13.0 #304

vsmart opened this issue Mar 7, 2019 · 0 comments
Labels

Comments

@vsmart
Copy link
Contributor

vsmart commented Mar 7, 2019

Since watchbot 4.13.0 it is possible to use watchbot with a fifo queue and the message parsing was changed to accomodate this. This broke allowing empty sns subjects, which was originally introduced in #240.

From lib/message.js:

// If the Watchbot instance uses a regular SQS queue, sqsMessage
// will have come from an SNS topic, so the body will be a JSON object
// with Message and Subject properties.
//
// If the SQS message is for a FIFO queue, it did not come from SNS
// so will not have the same structure and might not be JSON
// parseable. If it is, we'll parse it; if not, we'll just pass it on.

We try to see if the message is parseable here:

try {
  const parsedBody = JSON.parse(sqsMessage.Body);
  if (parsedBody.Subject)  {
    envMessage = parsedBody.Message;
    envSubject = parsedBody.Subject;
  }
} catch (error) {
  // JSON-parsing failure means we can leave the body as it is.
}

However, this will fail if the message does not contain a subject. The subject is not a required key, and so even a standard sns message that came through a non-fifo queue but for some reason does not contain a Subject key will not be parsed as expected.

I ran into this this week with two separate repositories while upgrading them to the latest watchbot version - in one case, I was sending an SNS message via the console and did not specify a Subject (again, not a required parameter), and in another case, I was sending the message using code like this:

sns
    .publish({
      Message: "my message",
      TopicArn: "arn:aws:sns..." 
})

... without specifying a Subject.

Next actions

I could imagine two possible fixes, but I need some more context on whether those might break something else 🤔

  1. Explicitly check that option.fifo === true in the try statement - since if I understand correctly, this is only needed for fifo queues.
  2. remove the if parsedBody.Subject clause and always assign message to parseBody.Message if parsedBody.Message exists and the json parsing was successful

cc @mapbox/platform

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

No branches or pull requests

1 participant