fix: headers are appended to existing one (#2335) (#2357)

* fix: headers are appended to existing one (#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 (#2335)

* test: review DRY

* fix: review lint

Co-authored-by: Valentin Marchaud <contact@vmarchaud.fr>
Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>

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);
   });
 

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<string,string>;
+    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<string,string>;
+      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();
+        });
+      });
+    });
+  });
+});