From 83f7867b1aa945063c9a2dd1f1e6372d2935d605 Mon Sep 17 00:00:00 2001 From: adorin Date: Wed, 13 Jan 2021 22:01:53 +0100 Subject: [PATCH] Fix push gateway attempting to write promise instead of awaiting (#419) * Added tests for push gateway awaiting on registry.metrics() * Fixed push gateway not awaiting on registry.metrics() * Added changelog for #419 Co-authored-by: Dorian More Co-authored-by: Zach Bjornson --- CHANGELOG.md | 2 ++ lib/pushgateway.js | 14 ++++++++++++-- test/pushgatewayTest.js | 41 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d82f6a8..e56d037c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ project adheres to [Semantic Versioning](http://semver.org/). ### Changed +- fix: push client attempting to write Promise + ### Added ## [13.0.0] - 2020-12-16 diff --git a/lib/pushgateway.js b/lib/pushgateway.js index 71d647ab..09f7f955 100644 --- a/lib/pushgateway.js +++ b/lib/pushgateway.js @@ -75,9 +75,19 @@ function useGateway(method, job, groupings, callback) { }); if (method !== 'DELETE') { - req.write(this.registry.metrics()); + this.registry + .metrics() + .then(metrics => { + req.write(metrics); + req.end(); + }) + .catch(err => { + req.end(); + callback(err); + }); + } else { + req.end(); } - req.end(); } function generateGroupings(groupings) { diff --git a/test/pushgatewayTest.js b/test/pushgatewayTest.js index 3cb7bb5f..5085b98c 100644 --- a/test/pushgatewayTest.js +++ b/test/pushgatewayTest.js @@ -1,10 +1,11 @@ 'use strict'; -const mockHttp = jest.fn().mockReturnValue({ +const httpMocks = { on: jest.fn(), end: jest.fn(), write: jest.fn(), -}); +}; +const mockHttp = jest.fn().mockReturnValue(httpMocks); jest.mock('http', () => { return { @@ -12,6 +13,11 @@ jest.mock('http', () => { }; }); +const payload = `# HELP test test +# TYPE test counter +test 100 +`; + describe('pushgateway', () => { const Pushgateway = require('../index').Pushgateway; const register = require('../index').register; @@ -47,6 +53,16 @@ describe('pushgateway', () => { expect(invocation.method).toEqual('POST'); expect(invocation.path).toEqual('/metrics/job/testJob/key/va%26lue'); }); + + it('should write the payload', () => { + instance.pushAdd({ jobName: 'testJob' }); + + return instance.registry.metrics().then(() => { + expect(httpMocks.write).toBeCalledTimes(1); + expect(httpMocks.end).toBeCalledTimes(1); + expect(httpMocks.write).toHaveBeenCalledWith(payload); + }); + }); }); describe('push', () => { @@ -67,6 +83,16 @@ describe('pushgateway', () => { expect(invocation.method).toEqual('PUT'); expect(invocation.path).toEqual('/metrics/job/test%26Job'); }); + + it('should write the payload', () => { + instance.push({ jobName: 'test&Job' }); + + return instance.registry.metrics().then(() => { + expect(httpMocks.write).toBeCalledTimes(1); + expect(httpMocks.end).toBeCalledTimes(1); + expect(httpMocks.write).toHaveBeenCalledWith(payload); + }); + }); }); describe('delete', () => { @@ -78,6 +104,13 @@ describe('pushgateway', () => { expect(invocation.method).toEqual('DELETE'); expect(invocation.path).toEqual('/metrics/job/testJob'); }); + + it('should not write a payload', () => { + instance.delete({ jobName: 'testJob' }); + + expect(httpMocks.end).toBeCalledTimes(1); + expect(httpMocks.write).toBeCalledTimes(0); + }); }); describe('when using basic authentication', () => { @@ -145,6 +178,8 @@ describe('pushgateway', () => { register.clear(); }); beforeEach(() => { + httpMocks.write.mockClear(); + httpMocks.end.mockClear(); registry = undefined; instance = new Pushgateway('http://192.168.99.100:9091'); const promClient = require('../index'); @@ -158,6 +193,8 @@ describe('pushgateway', () => { mockHttp.mockClear(); }); beforeEach(() => { + httpMocks.write.mockClear(); + httpMocks.end.mockClear(); registry = new Registry(); instance = new Pushgateway('http://192.168.99.100:9091', null, registry); const promeClient = require('../index');