Skip to content

Commit

Permalink
feat(node): Support tunnel option for ANR (#11163)
Browse files Browse the repository at this point in the history
Closes #11159

This PR also modifies `getEnvelopeEndpointWithUrlEncodedAuth` which had
a suggested simplification for v8.

I ignored the suggestion because `{ tunnel, _metadata }` feels like a
very "internal" API. Sure, this is convenient because it lets you pass
the client options directly but it doesn't sit right as a user facing
API.
  • Loading branch information
timfish committed Mar 18, 2024
1 parent 4bbb853 commit b9e44b9
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 36 deletions.
17 changes: 2 additions & 15 deletions packages/core/src/api.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ClientOptions, DsnComponents, DsnLike, SdkInfo } from '@sentry/types';
import type { DsnComponents, DsnLike, SdkInfo } from '@sentry/types';
import { dsnToString, makeDsn, urlEncode } from '@sentry/utils';

const SENTRY_API_VERSION = '7';
Expand Down Expand Up @@ -31,20 +31,7 @@ function _encodedAuth(dsn: DsnComponents, sdkInfo: SdkInfo | undefined): string
*
* Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
*/
export function getEnvelopeEndpointWithUrlEncodedAuth(
dsn: DsnComponents,
// TODO (v8): Remove `tunnelOrOptions` in favor of `options`, and use the substitute code below
// options: ClientOptions = {} as ClientOptions,
tunnelOrOptions: string | ClientOptions = {} as ClientOptions,
): string {
// TODO (v8): Use this code instead
// const { tunnel, _metadata = {} } = options;
// return tunnel ? tunnel : `${_getIngestEndpoint(dsn)}?${_encodedAuth(dsn, _metadata.sdk)}`;

const tunnel = typeof tunnelOrOptions === 'string' ? tunnelOrOptions : tunnelOrOptions.tunnel;
const sdkInfo =
typeof tunnelOrOptions === 'string' || !tunnelOrOptions._metadata ? undefined : tunnelOrOptions._metadata.sdk;

export function getEnvelopeEndpointWithUrlEncodedAuth(dsn: DsnComponents, tunnel?: string, sdkInfo?: SdkInfo): string {
return tunnel ? tunnel : `${_getIngestEndpoint(dsn)}?${_encodedAuth(dsn, sdkInfo)}`;
}

Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
}

if (this._dsn) {
const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options);
const url = getEnvelopeEndpointWithUrlEncodedAuth(
this._dsn,
options.tunnel,
options._metadata ? options._metadata.sdk : undefined,
);
this._transport = options.transport({
recordDroppedEvent: this.recordDroppedEvent.bind(this),
...options.transportOptions,
Expand Down
26 changes: 12 additions & 14 deletions packages/core/test/lib/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type { ClientOptions, DsnComponents } from '@sentry/types';
import type { DsnComponents, SdkInfo } from '@sentry/types';
import { makeDsn } from '@sentry/utils';

import { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from '../../src/api';

const ingestDsn = 'https://abc@xxxx.ingest.sentry.io:1234/subpath/123';
const dsnPublic = 'https://abc@sentry.io:1234/subpath/123';
const tunnel = 'https://hello.com/world';
const _metadata = { sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } } as ClientOptions['_metadata'];
const sdkInfo = { name: 'sentry.javascript.browser', version: '12.31.12' };

const dsnPublicComponents = makeDsn(dsnPublic)!;

Expand All @@ -17,36 +17,34 @@ describe('API', () => {
"doesn't include `sentry_client` when called with only DSN",
dsnPublicComponents,
undefined,
undefined,
'https://sentry.io:1234/subpath/api/123/envelope/?sentry_key=abc&sentry_version=7',
],
['uses `tunnel` value when called with `tunnel` as string', dsnPublicComponents, tunnel, tunnel],
['uses `tunnel` value when called with `tunnel` option', dsnPublicComponents, tunnel, undefined, tunnel],
[
'uses `tunnel` value when called with `tunnel` in options',
'uses `tunnel` value when called with `tunnel` and `sdkInfo` options',
dsnPublicComponents,
{ tunnel } as ClientOptions,
tunnel,
],
[
'uses `tunnel` value when called with `tunnel` and `_metadata` in options',
dsnPublicComponents,
{ tunnel, _metadata } as ClientOptions,
sdkInfo,
tunnel,
],
[
'includes `sentry_client` when called with `_metadata` in options and no tunnel',
'includes `sentry_client` when called with `sdkInfo` in options and no tunnel',
dsnPublicComponents,
{ _metadata } as ClientOptions,
undefined,
sdkInfo,
'https://sentry.io:1234/subpath/api/123/envelope/?sentry_key=abc&sentry_version=7&sentry_client=sentry.javascript.browser%2F12.31.12',
],
])(
'%s',
(
_testName: string,
dsnComponents: DsnComponents,
tunnelOrOptions: string | ClientOptions | undefined,
tunnel: string | undefined,
sdkInfo: SdkInfo | undefined,
expected: string,
) => {
expect(getEnvelopeEndpointWithUrlEncodedAuth(dsnComponents, tunnelOrOptions)).toBe(expected);
expect(getEnvelopeEndpointWithUrlEncodedAuth(dsnComponents, tunnel, sdkInfo)).toBe(expected);
},
);
});
Expand Down
1 change: 1 addition & 0 deletions packages/node-experimental/src/integrations/anr/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface WorkerStartData extends AnrIntegrationOptions {
debug: boolean;
sdkMetadata: SdkMetadata;
dsn: DsnComponents;
tunnel: string | undefined;
release: string | undefined;
environment: string;
dist: string | undefined;
Expand Down
1 change: 1 addition & 0 deletions packages/node-experimental/src/integrations/anr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ async function _startWorker(client: NodeClient, _options: Partial<AnrIntegration
const options: WorkerStartData = {
debug: logger.isEnabled(),
dsn,
tunnel: initOptions.tunnel,
environment: initOptions.environment || 'production',
release: initOptions.release,
dist: initOptions.dist,
Expand Down
12 changes: 6 additions & 6 deletions packages/node-experimental/src/integrations/anr/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function log(msg: string): void {
}
}

const url = getEnvelopeEndpointWithUrlEncodedAuth(options.dsn);
const url = getEnvelopeEndpointWithUrlEncodedAuth(options.dsn, options.tunnel, options.sdkMetadata.sdk);
const transport = makeNodeTransport({
url,
recordDroppedEvent: () => {
Expand All @@ -47,7 +47,7 @@ async function sendAbnormalSession(): Promise<void> {
log('Sending abnormal session');
updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' });

const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata);
const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata, options.tunnel);
// Log the envelope so to aid in testing
log(JSON.stringify(envelope));

Expand Down Expand Up @@ -119,15 +119,15 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext):
tags: options.staticTags,
};

const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata);
// Log the envelope so to aid in testing
const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata, options.tunnel);
// Log the envelope to aid in testing
log(JSON.stringify(envelope));

await transport.send(envelope);
await transport.flush(2000);

// Delay for 5 seconds so that stdio can flush in the main event loop ever restarts.
// This is mainly for the benefit of logging/debugging issues.
// Delay for 5 seconds so that stdio can flush if the main event loop ever restarts.
// This is mainly for the benefit of logging or debugging.
setTimeout(() => {
process.exit(0);
}, 5_000);
Expand Down

0 comments on commit b9e44b9

Please sign in to comment.