-
-
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
lforst 7bc6975
move rate limits header
lforst 40d37d7
Update todo
lforst 6019f60
Simplify request
lforst b1d6a60
Add first few test cases
lforst 5b9cfc6
Fix behaviour of no_proxy env var
lforst 0f312ce
Finalize tests
lforst 9194e85
Add documentation
lforst a159aaa
Rearrange imported type
lforst f166ea0
Import url to fix older node versions
lforst a4160c7
Rename options type of node transport
lforst 97f8aad
Update node backend to use new transport
lforst 47caf0b
Fix url to include query params
lforst 44f54b8
Unify http and https transport
lforst 3249d42
Add todo for v7 to rename imports
lforst 3ecd702
Set keepAlive to false and add comment to true it in v7
lforst 53e0fc0
Update keepAlive todo
lforst dd939ff
Directly pass object into makeNodeTransport
lforst 339062c
Clarify proxy order for http
lforst 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.