librato / statsd-librato-backend

A StatsD backend that sends metrics to Librato Metrics
MIT License
76 stars 34 forks source link

Avoid crash when measure time is too far in the past #67

Closed bluemaex closed 7 years ago

bluemaex commented 7 years ago

If the API responds with an HTTP 400 because the measure time is too far in the past it crashes the entire statsd server.

3 Mar 21:45:46 - LOG_ERR: Failed to post to Librato: HTTP 400: {"errors":{"params":{"measure_time":["is too far in the past"]}},"request_time":1488585496}
/statsd/node_modules/statsd-librato-backend/lib/librato.js:95
          if (meta.errors && meta.errors.params && meta.errors.params.type.length) {
                                                                          ^

TypeError: Cannot read property 'length' of undefined
    at IncomingMessage.<anonymous> (/statsd/node_modules/statsd-librato-backend/lib/librato.js:95:75)
    at emitOne (events.js:96:13)
    at IncomingMessage.emit (events.js:188:7)
    at IncomingMessage.Readable.read (_stream_readable.js:381:10)
    at flow (_stream_readable.js:761:34)
    at resume_ (_stream_readable.js:743:3)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)

This additional checks fixes this error.

bryanmikaelian commented 7 years ago

@bluemaex Thanks for reporting this. Might be possible that something recently changed in our API to cause this.

Out of curiosity, is your Librato account enabled for tag support? Or are you still using the legacy version (0.1.7) of this library?

bluemaex commented 7 years ago

@bryanmikaelian no, I stumpled upon this bug on the published npm version 2.0.2.

As for the the tag support, I am not completely sure - I don't think so because our account is pretty old. I will check this tomorrow, though!

bryanmikaelian commented 7 years ago

@bluemaex Perfect. We'll dig on our end too and get back to you by tomorrow.

bluemaex commented 7 years ago

So I think we are still on the sources plan - The raw metrics page says: Showing 1-3 of 3 Sources

Thanks for digging in too!

bryanmikaelian commented 7 years ago

@bluemaex Thanks for the info! Did some digging and this is definitely an issue with the backend plugin. The type param isn't always in the error message from our legacy API and definitely won't be there in the tag-based API error messages.

Appreciate the fix!

till commented 7 years ago

@bryanmikaelian can we also have a new release?

Ping @gilleyj this needs to be included in the new container.

bryanmikaelian commented 7 years ago

@till Yup. 2.0.4 should be live on NPM

bluemaex commented 7 years ago

Thanks @bryanmikaelian!