From cb06b788b27ff080326b74bf4c3d08f419a06529 Mon Sep 17 00:00:00 2001 From: Niko Achilles Kokkinos Date: Wed, 21 Jul 2021 18:57:23 +0300 Subject: [PATCH] fix: headers are appended to existing one (open-telemetry#2335) (#2357) * fix: headers are appended to existing one (open-telemetry#2335) * test(browser-util): added 2 more test-cases refactored desc, it wording * test: split tests review * fix: review headers are appended to existing one (open-telemetry#2335) * test: review DRY * fix: review lint Co-authored-by: Valentin Marchaud Co-authored-by: Daniel Dyla --- .../src/platform/browser/util.ts | 13 +- .../test/browser/util.test.ts | 128 ++++++++++++++++++ 2 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 packages/opentelemetry-exporter-collector/test/browser/util.test.ts diff --git a/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts b/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts index a695ee6944..5c57517235 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/browser/util.ts @@ -55,9 +55,16 @@ export function sendWithXhr( ) { const xhr = new XMLHttpRequest(); xhr.open('POST', url); - xhr.setRequestHeader('Accept', 'application/json'); - xhr.setRequestHeader('Content-Type', 'application/json'); - Object.entries(headers).forEach(([k, v]) => { + + const defaultHeaders = { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + }; + + Object.entries({ + ...defaultHeaders, + ...headers, + }).forEach(([k, v]) => { xhr.setRequestHeader(k, v); }); diff --git a/packages/opentelemetry-exporter-collector/test/browser/util.test.ts b/packages/opentelemetry-exporter-collector/test/browser/util.test.ts new file mode 100644 index 0000000000..ecb6c4c801 --- /dev/null +++ b/packages/opentelemetry-exporter-collector/test/browser/util.test.ts @@ -0,0 +1,128 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as sinon from 'sinon'; +import { sendWithXhr } from '../../src/platform/browser/util'; +import { ensureHeadersContain } from '../helper'; + +describe('util - browser', () => { + let server: any; + const body = ''; + const url = ''; + + let onSuccessStub: sinon.SinonStub; + let onErrorStub: sinon.SinonStub; + + beforeEach(() => { + onSuccessStub = sinon.stub(); + onErrorStub = sinon.stub(); + server = sinon.fakeServer.create(); + }); + + afterEach(() => { + server.restore(); + sinon.restore(); + }); + + describe('when XMLHTTPRequest is used', () => { + let expectedHeaders: Record; + beforeEach(()=>{ + expectedHeaders = { + // ;charset=utf-8 is applied by sinon.fakeServer + 'Content-Type': 'application/json;charset=utf-8', + 'Accept': 'application/json', + } + }); + describe('and Content-Type header is set', () => { + beforeEach(()=>{ + const explicitContentType = { + 'Content-Type': 'application/json', + }; + sendWithXhr(body, url, explicitContentType, onSuccessStub, onErrorStub); + }); + it('Request Headers should contain "Content-Type" header', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, expectedHeaders); + done(); + }); + }); + it('Request Headers should contain "Accept" header', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, expectedHeaders); + done(); + }); + }); + }); + + describe('and empty headers are set', () => { + beforeEach(()=>{ + const emptyHeaders = {}; + sendWithXhr(body, url, emptyHeaders, onSuccessStub, onErrorStub); + }); + it('Request Headers should contain "Content-Type" header', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, expectedHeaders); + done(); + }); + }); + it('Request Headers should contain "Accept" header', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, expectedHeaders); + done(); + }); + }); + }); + describe('and custom headers are set', () => { + let customHeaders: Record; + beforeEach(()=>{ + customHeaders = { aHeader: 'aValue', bHeader: 'bValue' }; + sendWithXhr(body, url, customHeaders, onSuccessStub, onErrorStub); + }); + it('Request Headers should contain "Content-Type" header', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, expectedHeaders); + done(); + }); + }); + it('Request Headers should contain "Accept" header', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, expectedHeaders); + done(); + }); + }); + it('Request Headers should contain custom headers', done => { + + setTimeout(() => { + const { requestHeaders } = server.requests[0]; + ensureHeadersContain(requestHeaders, customHeaders); + done(); + }); + }); + }); + }); +});