-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(node): Add new v7 http/s Transports #4781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
ee01adc
initial draft for v7 node transports
7bc6975
move rate limits header
40d37d7
Update todo
6019f60
Simplify request
b1d6a60
Add first few test cases
5b9cfc6
Fix behaviour of no_proxy env var
0f312ce
Finalize tests
9194e85
Add documentation
a159aaa
Rearrange imported type
f166ea0
Import url to fix older node versions
a4160c7
Rename options type of node transport
97f8aad
Update node backend to use new transport
47caf0b
Fix url to include query params
44f54b8
Unify http and https transport
3249d42
Add todo for v7 to rename imports
3ecd702
Set keepAlive to false and add comment to true it in v7
53e0fc0
Update keepAlive todo
dd939ff
Directly pass object into makeNodeTransport
339062c
Clarify proxy order for http
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
export { BaseTransport } from './base'; | ||
export { HTTPTransport } from './http'; | ||
export { HTTPSTransport } from './https'; | ||
export { makeNodeTransport, NodeTransportOptions } from './new'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
import { | ||
BaseTransportOptions, | ||
createTransport, | ||
NewTransport, | ||
TransportMakeRequestResponse, | ||
TransportRequest, | ||
TransportRequestExecutor, | ||
} from '@sentry/core'; | ||
import { eventStatusFromHttpCode } from '@sentry/utils'; | ||
import * as http from 'http'; | ||
import * as https from 'https'; | ||
import { URL } from 'url'; | ||
|
||
import { HTTPModule } from './base/http-module'; | ||
|
||
// TODO(v7): | ||
// - Rename this file "transport.ts" | ||
// - Move this file one folder upwards | ||
// - Delete "transports" folder | ||
// OR | ||
// - Split this file up and leave it in the transports folder | ||
|
||
export interface NodeTransportOptions extends BaseTransportOptions { | ||
/** Define custom headers */ | ||
headers?: Record<string, string>; | ||
/** Set a proxy that should be used for outbound requests. */ | ||
proxy?: string; | ||
/** HTTPS proxy CA certificates */ | ||
caCerts?: string | Buffer | Array<string | Buffer>; | ||
/** Custom HTTP module. Defaults to the native 'http' and 'https' modules. */ | ||
httpModule?: HTTPModule; | ||
} | ||
|
||
/** | ||
* Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry. | ||
*/ | ||
export function makeNodeTransport(options: NodeTransportOptions): NewTransport { | ||
const urlSegments = new URL(options.url); | ||
const isHttps = urlSegments.protocol === 'https:'; | ||
|
||
// Proxy prioritization: http => `options.proxy` | `process.env.http_proxy` | ||
// Proxy prioritization: https => `options.proxy` | `process.env.https_proxy` | `process.env.http_proxy` | ||
lforst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const proxy = applyNoProxyOption( | ||
urlSegments, | ||
options.proxy || (isHttps ? process.env.https_proxy : undefined) || process.env.http_proxy, | ||
); | ||
|
||
const nativeHttpModule = isHttps ? https : http; | ||
|
||
// TODO(v7): Evaluate if we can set keepAlive to true. This would involve testing for memory leaks in older node | ||
// versions(>= 8) as they had memory leaks when using it: #2555 | ||
const agent = proxy | ||
? (new (require('https-proxy-agent'))(proxy) as http.Agent) | ||
: new nativeHttpModule.Agent({ keepAlive: false, maxSockets: 30, timeout: 2000 }); | ||
lforst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent); | ||
return createTransport({ bufferSize: options.bufferSize }, requestExecutor); | ||
} | ||
|
||
/** | ||
* Honors the `no_proxy` env variable with the highest priority to allow for hosts exclusion. | ||
* | ||
* @param transportUrl The URL the transport intends to send events to. | ||
* @param proxy The client configured proxy. | ||
* @returns A proxy the transport should use. | ||
*/ | ||
function applyNoProxyOption(transportUrlSegments: URL, proxy: string | undefined): string | undefined { | ||
const { no_proxy } = process.env; | ||
|
||
const urlIsExemptFromProxy = | ||
no_proxy && | ||
no_proxy | ||
.split(',') | ||
.some( | ||
exemption => transportUrlSegments.host.endsWith(exemption) || transportUrlSegments.hostname.endsWith(exemption), | ||
); | ||
|
||
if (urlIsExemptFromProxy) { | ||
return undefined; | ||
} else { | ||
return proxy; | ||
} | ||
} | ||
|
||
/** | ||
* Creates a RequestExecutor to be used with `createTransport`. | ||
*/ | ||
function createRequestExecutor( | ||
options: NodeTransportOptions, | ||
httpModule: HTTPModule, | ||
agent: http.Agent, | ||
): TransportRequestExecutor { | ||
const { hostname, pathname, port, protocol, search } = new URL(options.url); | ||
|
||
return function makeRequest(request: TransportRequest): Promise<TransportMakeRequestResponse> { | ||
return new Promise((resolve, reject) => { | ||
const req = httpModule.request( | ||
{ | ||
method: 'POST', | ||
agent, | ||
headers: options.headers, | ||
hostname, | ||
path: `${pathname}${search}`, | ||
port, | ||
protocol, | ||
ca: options.caCerts, | ||
}, | ||
res => { | ||
res.on('data', () => { | ||
// Drain socket | ||
}); | ||
|
||
res.on('end', () => { | ||
// Drain socket | ||
}); | ||
|
||
const statusCode = res.statusCode ?? 500; | ||
const status = eventStatusFromHttpCode(statusCode); | ||
|
||
res.setEncoding('utf8'); | ||
|
||
// "Key-value pairs of header names and values. Header names are lower-cased." | ||
// https://nodejs.org/api/http.html#http_message_headers | ||
const retryAfterHeader = res.headers['retry-after'] ?? null; | ||
const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null; | ||
|
||
resolve({ | ||
headers: { | ||
'retry-after': retryAfterHeader, | ||
'x-sentry-rate-limits': Array.isArray(rateLimitsHeader) ? rateLimitsHeader[0] : rateLimitsHeader, | ||
}, | ||
reason: status, | ||
statusCode: statusCode, | ||
}); | ||
}, | ||
); | ||
|
||
req.on('error', reject); | ||
req.end(request.body); | ||
}); | ||
}; | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.