-
Notifications
You must be signed in to change notification settings - Fork 97
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
feat: introduces X-Request-ID header to more easily identify retried requests #678
Conversation
size-limit report 📦
|
Co-authored-by: Eric Swanson <64809312+ericswanson-dfinity@users.noreply.github.com>
…to SDK-880-header-nonce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but left one comment on the use of btoa
}, | ||
body: cbor.encode(expectedRequest), | ||
}); | ||
const call1 = calls[0][0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const call1 = calls[0][0]; | |
const [call1, call2] = calls[0] |
@@ -293,10 +293,10 @@ export class HttpAgent implements Agent { | |||
request: { | |||
body: null, | |||
method: 'POST', | |||
headers: { | |||
headers: new Headers({ | |||
'Content-Type': 'application/cbor', | |||
...(this._credentials ? { Authorization: 'Basic ' + btoa(this._credentials) } : {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btoa
is deprecated in node. would this ever run in a node context? if it were, better to detect the environment and use Buffer
instead
if (isBrowserEnv()) {
window.btoa(this._credentials);
}
if (isNodeEnv()) {
Buffer.from(this._credentials).toString('base64')
}
Description
During the SNS-1 launch retrospective, we identified the difficulty in identifying retried requests as a pain point.
Adding a unique request id header (using the standard key, despite the similarity to the Request ID specified for checking the status of updates in the Internet Computer Specification
Fixes # SDK-880
How Has This Been Tested?
new unit tests
Checklist: