diff --git a/CHANGELOG.md b/CHANGELOG.md index d838fa4cc94..e67265f13a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ fix(opentelemetry-resources): do not discard OTEL_RESOURCE_ATTRIBUTES when it co ### :house: Internal +* test(exporter-zipkin): fix broken browser test assertions and add missing coverage [#6566](https://github.com/open-telemetry/opentelemetry-js/pull/6566) @overbalance + ## 2.6.1 ### :bug: Bug Fixes diff --git a/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts index 23f402947f1..e216a522da2 100644 --- a/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/browser/zipkin.test.ts @@ -4,6 +4,7 @@ */ import { + ExportResultCode, setGlobalErrorHandler, loggingErrorHandler, } from '@opentelemetry/core'; @@ -24,7 +25,7 @@ describe('Zipkin Exporter - web', () => { let zipkinExporter: ZipkinExporter; let zipkinConfig: zipkinTypes.ExporterConfig = {}; let spySend: sinon.SinonSpy; - let spyBeacon: sinon.SinonSpy; + let spyBeacon: sinon.SinonStub; let spans: ReadableSpan[]; beforeEach(() => { @@ -59,6 +60,18 @@ describe('Zipkin Exporter - web', () => { done(); }); }); + + // sendBeacon returning false signals the browser refused to queue the + // request (e.g. payload too large). The export callback receives FAILED. + it('should report FAILED when sendBeacon returns false', done => { + spyBeacon.returns(false); + zipkinExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + assert.ok(result.error); + assert.ok(result.error!.message.includes('sendBeacon - cannot send')); + done(); + }); + }); }); describe('when "sendBeacon" is NOT available', () => { @@ -84,15 +97,40 @@ describe('Zipkin Exporter - web', () => { done(); }); }); + + // Network-level errors (DNS failure, CORS, connection refused) trigger + // xhr.onerror, which calls globalErrorHandler. The export callback + // receives FAILED without an error object. + it('should call globalErrorHandler on network error', done => { + const errorHandlerSpy = sinon.spy(); + setGlobalErrorHandler(errorHandlerSpy); + + zipkinExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + assert.strictEqual(result.error, undefined); + assert.strictEqual(errorHandlerSpy.callCount, 1); + assert.ok( + errorHandlerSpy.args[0][0].message.includes('Zipkin request error') + ); + setGlobalErrorHandler(loggingErrorHandler()); + done(); + }); + + setTimeout(() => { + const request = server.requests[0]; + request.onerror(new ProgressEvent('error')); + }); + }); }); - describe('should use url defined in environment', () => { + describe('should use url from config', () => { let server: any; const endpointUrl = 'http://localhost:9412'; beforeEach(() => { (window.navigator as any).sendBeacon = false; - (window as any).OTEL_EXPORTER_ZIPKIN_ENDPOINT = endpointUrl; - zipkinExporter = new ZipkinExporter(zipkinConfig); + // Browser getStringFromEnv() always returns undefined, so env-based + // URL override does not work. Use config.url instead. + zipkinExporter = new ZipkinExporter({ url: endpointUrl }); server = sinon.fakeServer.create(); }); afterEach(() => { @@ -104,7 +142,7 @@ describe('Zipkin Exporter - web', () => { setTimeout(() => { const request = server.requests[0]; - assert.ok(request.url, endpointUrl); + assert.strictEqual(request.url, endpointUrl); const body = request.requestBody; const json = JSON.parse(body) as any; ensureSpanIsCorrect(json[0]); @@ -184,17 +222,19 @@ describe('Zipkin Exporter - web', () => { }); }); - it('should call globalErrorHandler on error', () => { - const errorHandlerSpy = sinon.spy(); - setGlobalErrorHandler(errorHandlerSpy); - - zipkinExporter.export(spans, () => { - const [[error]] = errorHandlerSpy.args; - assert.strictEqual(errorHandlerSpy.callCount, 1); - assert.ok(error.message.includes('Zipkin request error')); - - //reset global error handler - setGlobalErrorHandler(loggingErrorHandler()); + // HTTP 400 triggers onreadystatechange, not xhr.onerror, so + // globalErrorHandler is not called. The export callback receives FAILED. + // Custom headers force XHR usage regardless of sendBeacon availability. + it('should report FAILED export result on HTTP error', done => { + zipkinExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + assert.ok(result.error); + assert.ok( + result.error!.message.includes( + 'Got unexpected status code from zipkin: 400' + ) + ); + done(); }); setTimeout(() => { @@ -223,17 +263,18 @@ describe('Zipkin Exporter - web', () => { }); }); - it('should call globalErrorHandler on error', () => { - const errorHandlerSpy = sinon.spy(); - setGlobalErrorHandler(errorHandlerSpy); - - zipkinExporter.export(spans, () => { - const [[error]] = errorHandlerSpy.args; - assert.strictEqual(errorHandlerSpy.callCount, 1); - assert.ok(error.message.includes('sendBeacon - cannot send')); - - //reset global error handler - setGlobalErrorHandler(loggingErrorHandler()); + // sendBeacon is false here so XHR is used. HTTP 400 triggers + // onreadystatechange (not onerror), so the export callback receives FAILED. + it('should report FAILED export result on HTTP error', done => { + zipkinExporter.export(spans, result => { + assert.strictEqual(result.code, ExportResultCode.FAILED); + assert.ok(result.error); + assert.ok( + result.error!.message.includes( + 'Got unexpected status code from zipkin: 400' + ) + ); + done(); }); setTimeout(() => {