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 <[email protected]> Co-authored-by: Zach Bjornson <[email protected]>
siimon · Jan 13, 2021 · 83f7867 · 83f7867
1 parent bec8067
commit 83f7867
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 4 deletions.
diff --git 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
@@ -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
@@ -1,17 +1,23 @@
 '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 {
 		request: mockHttp,
 	};
 });
 
+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');