[SDK-3975] Log all HTTP activity #1581
Merged
Conversation
This file contains 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
It’s just an implementation detail of the web class.
It’s just an implementation detail of the Node class.
I’m going to introduce a platform-agnostic class called Http.
github-actions
bot
temporarily deployed
to
staging/pull/1581/features
github-actions
bot
temporarily deployed
to
staging/pull/1581/typedoc
github-actions
bot
temporarily deployed
to
staging/pull/1581/bundle-report
This will hold HTTP-related functionality that’s shared across platforms, such fallback host usage or logging.
Makes it consistent with the Node version.
Looked into what types we pass, because I wanted to understand it better for some logging that I’m going to add. Thought I might as well update the types to reflect what I’d found out.
To avoid loss of information when unprintable characters (e.g. in MessagePack responses) get replaced (either by console.log or by the terminal, not sure which) with U+FFFD.
Else we often just see "[object Object]" in the logs.
Owen requested this during a review of #1526, which strips most of our logging from the modular variant of the SDK, but aims to still log all network activity. A bonus benefit is that we now also have logging for requests which get tried against different hosts, which we previously didn’t. The log statements are adaptations of the existing ones in resource.ts.
lawrence-forooghian
force-pushed
the
improve-http-logging
branch
from
0e73cfc
to
d43f965
Compare
owenpearson
approved these changes
Jan 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This addresses @owenpearson’s comment on #1526 by logging all of the SDK’s HTTP activity. It also introduces some refactors and improvements to related code. See commit messages for more details.