chore(various): Fix comments and docstrings (#5651)

lobsterkatie · web-flow · commit 754248253733 · 2022-08-30T11:49:30.000-07:00
Typo/grammar/clarification fixes in comments and docstrings. No behavior changes.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -1,6 +1,8 @@
 // Note: All paths are relative to the directory in which eslint is being run, rather than the directory where this file
 // lives
 
+// ESLint config docs: https://eslint.org/docs/user-guide/configuring/
+
 module.exports = {
   root: true,
   env: {
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -211,6 +211,8 @@ jobs:
 
   job_lint:
     name: Lint
+    # Even though the linter only checks source code, not built code, it needs the built code in order check that all
+    # inter-package dependencies resolve cleanly.
     needs: [job_get_metadata, job_build]
     timeout-minutes: 10
     runs-on: ubuntu-latest
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -2,13 +2,17 @@
   // Use IntelliSense to learn about possible Node.js debug attributes.
   // Hover to view descriptions of existing attributes.
   // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+  // For available variables, visit: https://code.visualstudio.com/docs/editor/variables-reference
   "version": "0.2.0",
   "inputs": [
     {
+      // Get the name of the package containing the file in the active tab.
       "id": "getPackageName",
       "type": "command",
       "command": "shellCommand.execute",
       "args": {
+        // Get the current file's absolute path, chop off everything up to and including the repo's `packages`
+        // directory, then split on `/` and take the first entry
         "command": "echo '${file}' | sed s/'.*sentry-javascript\\/packages\\/'// | grep --extended-regexp --only-matching --max-count 1 '[^\\/]+' | head -1",
         "cwd": "${workspaceFolder}",
         // normally `input` commands bring up a selector for the user, but given that there should only be one
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -514,7 +514,7 @@ Work in this release contributed by @7inspire, @jaeseokk, and @rchl. Thank you f
 This patch contains a breaking change for anyone setting the undocumented `rethrowAfterCapture` option for `@sentry/serverless`'s AWS wrapper to `false`, as its functionality has been removed. For backwards compatibility with anyone setting it to `true` (which is also the default), the option remains in the `WrapperOptions` type for now. It will be removed in the next major release, though, so we recommend removing it from your code.
 
 - ref(serverless): Remove `rethrowAfterCapture` use in AWS lambda wrapper (#4448)
-- fix(utils): Remove dom is casting (#4451)
+- fix(utils): Remove dom `is` casting (#4451)
 
 ## 6.17.1
 
diff --git a/jest/jest.config.js b/jest/jest.config.js
@@ -1,4 +1,5 @@
 module.exports = {
+  // this is the package root, even when tests are being run at the repo level
   rootDir: process.cwd(),
   collectCoverage: true,
   transform: {
diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts
@@ -86,7 +86,7 @@ export function createTransport(
       result => result,
       error => {
         if (error instanceof SentryError) {
-          __DEBUG_BUILD__ && logger.error('Skipped sending event due to full buffer');
+          __DEBUG_BUILD__ && logger.error('Skipped sending event because buffer is full.');
           recordEnvelopeLoss('queue_overflow');
           return resolvedSyncPromise();
         } else {
diff --git a/packages/gatsby/gatsby-browser.js b/packages/gatsby/gatsby-browser.js
@@ -9,8 +9,8 @@ exports.onClientEntry = function (_, pluginParams) {
     if (areOptionsDefined) {
       console.warn(
         'Sentry Logger [Warn]: The SDK was initialized in the Sentry config file, but options were found in the Gatsby config. ' +
-        'These have been ignored, merge them to the Sentry config if you want to use them.\n' +
-        'Learn more about the Gatsby SDK on https://docs.sentry.io/platforms/javascript/guides/gatsby/',
+          'These have been ignored. Merge them to the Sentry config if you want to use them.\n' +
+          'Learn more about the Gatsby SDK in https://docs.sentry.io/platforms/javascript/guides/gatsby/.',
       );
     }
     return;
@@ -19,7 +19,7 @@ exports.onClientEntry = function (_, pluginParams) {
   if (!areOptionsDefined) {
     console.error(
       'Sentry Logger [Error]: No config for the Gatsby SDK was found.\n' +
-      'Learn how to configure it on https://docs.sentry.io/platforms/javascript/guides/gatsby/',
+        'Learn how to configure it in https://docs.sentry.io/platforms/javascript/guides/gatsby/.',
     );
     return;
   }
diff --git a/packages/gatsby/test/gatsby-browser.test.ts b/packages/gatsby/test/gatsby-browser.test.ts
@@ -81,8 +81,8 @@ describe('onClientEntry', () => {
       // eslint-disable-next-line no-console
       expect((console.warn as jest.Mock).mock.calls[0]).toMatchInlineSnapshot(`
         Array [
-          "Sentry Logger [Warn]: The SDK was initialized in the Sentry config file, but options were found in the Gatsby config. These have been ignored, merge them to the Sentry config if you want to use them.
-        Learn more about the Gatsby SDK on https://docs.sentry.io/platforms/javascript/guides/gatsby/",
+          "Sentry Logger [Warn]: The SDK was initialized in the Sentry config file, but options were found in the Gatsby config. These have been ignored. Merge them to the Sentry config if you want to use them.
+        Learn more about the Gatsby SDK in https://docs.sentry.io/platforms/javascript/guides/gatsby/.",
         ]
       `);
       // eslint-disable-next-line no-console
@@ -98,7 +98,7 @@ describe('onClientEntry', () => {
       expect((console.error as jest.Mock).mock.calls[0]).toMatchInlineSnapshot(`
         Array [
           "Sentry Logger [Error]: No config for the Gatsby SDK was found.
-        Learn how to configure it on https://docs.sentry.io/platforms/javascript/guides/gatsby/",
+        Learn how to configure it in https://docs.sentry.io/platforms/javascript/guides/gatsby/.",
         ]
       `);
     });
diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts
@@ -441,11 +441,10 @@ export class Scope implements ScopeInterface {
   }
 
   /**
-   * Applies the current context and fingerprint to the event.
-   * Note that breadcrumbs will be added by the client.
-   * Also if the event has already breadcrumbs on it, we do not merge them.
+   * Applies data from the scope to the event and runs all event processors on it.
+   *
    * @param event Event
-   * @param hint May contain additional information about the original exception.
+   * @param hint Object containing additional information about the original exception, for use by the event processors.
    * @hidden
    */
   public applyToEvent(event: Event, hint: EventHint = {}): PromiseLike<Event | null> {
diff --git a/packages/hub/test/exports.test.ts b/packages/hub/test/exports.test.ts
@@ -106,6 +106,7 @@ describe('Top Level API', () => {
     });
 
     // NOTE: We left custom level as 2nd argument to not break the API. Should be removed and unified in v6.
+    // TODO: Before we release v8, check if this is still a thing
     test('Message with custom level', () => {
       const client: any = { captureMessage: jest.fn(async () => Promise.resolve()) };
       getCurrentHub().withScope(() => {
diff --git a/packages/integrations/rollup.bundle.config.js b/packages/integrations/rollup.bundle.config.js
@@ -16,6 +16,7 @@ const baseBundleConfig = makeBaseBundleConfig({
 });
 
 // TODO We only need `commonjs` for localforage (used in the offline plugin). Once that's fixed, this can come out.
+// CommonJS plugin docs: https://github.com/rollup/plugins/tree/master/packages/commonjs
 baseBundleConfig.plugins = insertAt(baseBundleConfig.plugins, -2, commonjs());
 
 // this makes non-minified, minified, and minified-with-debug-logging versions of each bundle
diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts
@@ -22,15 +22,15 @@ export type NextConfigFunctionWithSentry = (
 ) => NextConfigObjectWithSentry;
 
 export type NextConfigObject = {
-  // custom webpack options
+  // Custom webpack options
   webpack?: WebpackConfigFunction;
-  // whether to build serverless functions for all pages, not just API routes
+  // Whether to build serverless functions for all pages, not just API routes. Removed in nextjs 12+.
   target?: 'server' | 'experimental-serverless-trace';
-  // the output directory for the built app (defaults to ".next")
+  // The output directory for the built app (defaults to ".next")
   distDir?: string;
-  // the root at which the nextjs app will be served (defaults to "/")
+  // The root at which the nextjs app will be served (defaults to "/")
   basePath?: string;
-  // config which will be available at runtime
+  // Config which will be available at runtime
   publicRuntimeConfig?: { [key: string]: unknown };
 };
 
@@ -39,6 +39,9 @@ export type UserSentryOptions = {
   // the plugin to be enabled, even in situations where it's not recommended.
   disableServerWebpackPlugin?: boolean;
   disableClientWebpackPlugin?: boolean;
+
+  // Use `hidden-source-map` for webpack `devtool` option, which strips the `sourceMappingURL` from the bottom of built
+  // JS files
   hideSourceMaps?: boolean;
 
   // Force webpack to apply the same transpilation rules to the SDK code as apply to user code. Helpful when targeting
@@ -64,9 +67,8 @@ export type NextConfigFunction = (phase: string, defaults: { defaultConfig: Next
  * Webpack config
  */
 
-// the format for providing custom webpack config in your nextjs options
+// The two possible formats for providing custom webpack config in `next.config.js`
 export type WebpackConfigFunction = (config: WebpackConfigObject, options: BuildContext) => WebpackConfigObject;
-
 export type WebpackConfigObject = {
   devtool?: string;
   plugins?: Array<WebpackPluginInstance | SentryWebpackPlugin>;
@@ -81,7 +83,7 @@ export type WebpackConfigObject = {
     rules: Array<WebpackModuleRule>;
   };
 } & {
-  // other webpack options
+  // Other webpack options
   [key: string]: unknown;
 };
 
diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts
@@ -29,7 +29,7 @@ export { SentryWebpackPlugin };
  * Sets:
  *   - `devtool`, to ensure high-quality sourcemaps are generated
  *   - `entry`, to include user's sentry config files (where `Sentry.init` is called) in the build
- *   - `plugins`, to add SentryWebpackPlugin (TODO: optional)
+ *   - `plugins`, to add SentryWebpackPlugin
  *
  * @param userNextConfig The user's existing nextjs config, as passed to `withSentryConfig`
  * @param userSentryWebpackPluginOptions The user's SentryWebpackPlugin config, as passed to `withSentryConfig`
diff --git a/packages/nextjs/test/run-integration-tests.sh b/packages/nextjs/test/run-integration-tests.sh
@@ -54,7 +54,7 @@ for NEXTJS_VERSION in 10 11 12; do
 
   echo "[nextjs@$NEXTJS_VERSION] Installing dependencies..."
   # set the desired version of next long enough to run yarn, and then restore the old version (doing the restoration now
-  # rather than during overall cleanup lets us look for "latest" in both loops)
+  # rather than during overall cleanup lets us look for "latest" in every loop)
   cp package.json package.json.bak
   if [[ $(uname) == "Darwin" ]]; then
     sed -i "" /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json
diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts
@@ -7,7 +7,7 @@ import { logAndExitProcess } from './utils/errorhandling';
 
 type OnFatalErrorHandler = (firstError: Error, secondError?: Error) => void;
 
-/** Global Promise Rejection handler */
+/** Global Exception handler */
 export class OnUncaughtException implements Integration {
   /**
    * @inheritDoc
diff --git a/packages/node/test/client.test.ts b/packages/node/test/client.test.ts
@@ -27,6 +27,7 @@ describe('NodeClient', () => {
       const requestSession = scope.getRequestSession();
       expect(requestSession!.status).toEqual('ok');
     });
+
     test('when autoSessionTracking is disabled -> requestStatus should not be set', () => {
       const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: false, release: '1.4' });
       client = new NodeClient(options);
@@ -42,6 +43,7 @@ describe('NodeClient', () => {
       const requestSession = scope.getRequestSession();
       expect(requestSession!.status).toEqual('ok');
     });
+
     test('when autoSessionTracking is enabled + requestSession status is Crashed -> requestStatus should not be overridden', () => {
       const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
       client = new NodeClient(options);
@@ -57,6 +59,7 @@ describe('NodeClient', () => {
       const requestSession = scope.getRequestSession();
       expect(requestSession!.status).toEqual('crashed');
     });
+
     test('when autoSessionTracking is enabled + error occurs within request bounds -> requestStatus should be set to Errored', () => {
       const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
       client = new NodeClient(options);
@@ -72,6 +75,7 @@ describe('NodeClient', () => {
       const requestSession = scope.getRequestSession();
       expect(requestSession!.status).toEqual('errored');
     });
+
     test('when autoSessionTracking is enabled + error occurs outside of request bounds -> requestStatus should not be set to Errored', () => {
       const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
       client = new NodeClient(options);
diff --git a/packages/serverless/src/awslambda.ts b/packages/serverless/src/awslambda.ts
@@ -250,7 +250,7 @@ export function wrapHandler<TEvent, TResult>(
   };
   let timeoutWarningTimer: NodeJS.Timeout;
 
-  // AWSLambda is like Express. It makes a distinction about handlers based on it's last argument
+  // AWSLambda is like Express. It makes a distinction about handlers based on its last argument
   // async (event) => async handler
   // async (event, context) => async handler
   // (event, context, callback) => sync handler
diff --git a/packages/tracing/src/index.bundle.ts b/packages/tracing/src/index.bundle.ts
@@ -75,10 +75,10 @@ const INTEGRATIONS = {
 };
 
 export { INTEGRATIONS as Integrations };
-// Though in this case exporting this separately in addition to exporting it as part of `Sentry.Integrations` doesn't
-// gain us any bundle size advantage (we're making the bundle here, not the user, and we can't leave anything out of
-// ours), it does bring the API for using the integration in line with that recommended for users bundling Sentry
-// themselves.
+// Though in this case exporting `BrowserTracing` separately (in addition to exporting it as part of
+// `Sentry.Integrations`) doesn't gain us any bundle size advantage (we're making the bundle here, not the user, and we
+// can't leave anything out of ours), it does bring the API for using the integration in line with that recommended for
+// users bundling Sentry themselves.
 export { BrowserTracing };
 
 // We are patching the global object with our hub extension methods
diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts
@@ -97,7 +97,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
    */
   flush(timeout?: number): PromiseLike<boolean>;
 
-  /** Returns an array of installed integrations on the client. */
+  /** Returns the client's instance of the given integration class, it any. */
   getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null;
 
   /** This is an internal function to setup all integrations that should run on the client */
diff --git a/packages/utils/README.md b/packages/utils/README.md
@@ -18,6 +18,6 @@
 
 ## General
 
-Common utilities used by the Sentry JavaScript SDKs. **Warning, only submodule imports are allowed here.** Also note,
-this function shouldn't be used externally, we will break them from time to time. This package is not part of our public
-API contract, we use them only internally.
+Common utilities used by the Sentry JavaScript SDKs.
+
+Note: This package is only meant to be used internally, and as such is not part of our public API contract and does not follow semver.
diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts
@@ -23,9 +23,15 @@ export function isError(wat: unknown): wat is Error {
       return isInstanceOf(wat, Error);
   }
 }
-
-function isBuiltin(wat: unknown, ty: string): boolean {
-  return objectToString.call(wat) === `[object ${ty}]`;
+/**
+ * Checks whether given value is an instance of the given built-in class.
+ *
+ * @param wat The value to be checked
+ * @param className
+ * @returns A boolean representing the result.
+ */
+function isBuiltin(wat: unknown, className: string): boolean {
+  return objectToString.call(wat) === `[object ${className}]`;
 }
 
 /**
diff --git a/packages/utils/src/normalize.ts b/packages/utils/src/normalize.ts
@@ -28,7 +28,7 @@ type ObjOrArray<T> = { [key: string]: T };
  * @param input The object to be normalized.
  * @param depth The max depth to which to normalize the object. (Anything deeper stringified whole.)
  * @param maxProperties The max number of elements or properties to be included in any single array or
- * object in the normallized output..
+ * object in the normallized output.
  * @returns A normalized version of the object, or `"**non-serializable**"` if any errors are thrown during normalization.
  */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
diff --git a/packages/utils/src/promisebuffer.ts b/packages/utils/src/promisebuffer.ts
@@ -42,7 +42,7 @@ export function makePromiseBuffer<T>(limit?: number): PromiseBuffer<T> {
    */
   function add(taskProducer: () => PromiseLike<T>): PromiseLike<T> {
     if (!isReady()) {
-      return rejectedSyncPromise(new SentryError('Not adding Promise due to buffer limit reached.'));
+      return rejectedSyncPromise(new SentryError('Not adding Promise because buffer limit was reached.'));
     }
 
     // start the task and add its promise to the queue
diff --git a/packages/utils/test/normalize.test.ts b/packages/utils/test/normalize.test.ts
@@ -184,7 +184,7 @@ describe('normalize()', () => {
     });
   });
 
-  describe('dont mutate and skip non-enumerables', () => {
+  describe("doesn't mutate the given object and skips non-enumerables", () => {
     test('simple object', () => {
       const circular = {
         foo: 1,
@@ -311,7 +311,7 @@ describe('normalize()', () => {
     });
   });
 
-  describe('changes unserializeable/global values/classes to its string representation', () => {
+  describe('changes unserializeable/global values/classes to their respective string representations', () => {
     test('primitive values', () => {
       expect(normalize(undefined)).toEqual('[undefined]');
       expect(normalize(NaN)).toEqual('[NaN]');
@@ -376,7 +376,7 @@ describe('normalize()', () => {
       });
     });
 
-    test('known Classes like Reacts SyntheticEvents', () => {
+    test("known classes like React's `SyntheticEvent`", () => {
       const obj = {
         foo: {
           nativeEvent: 'wat',
@@ -510,7 +510,7 @@ describe('normalize()', () => {
     });
   });
 
-  test('normalizes value on every iteration of decycle and takes care of things like Reacts SyntheticEvents', () => {
+  test("normalizes value on every iteration of decycle and takes care of things like React's `SyntheticEvent`", () => {
     const obj = {
       foo: {
         nativeEvent: 'wat',
diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts
@@ -134,7 +134,7 @@ describe('SyncPromise', () => {
     });
   });
 
-  test('calling the callback immediatly', () => {
+  test('calling the callback immediately', () => {
     expect.assertions(1);
 
     let foo: number = 1;
diff --git a/rollup/plugins/bundlePlugins.js b/rollup/plugins/bundlePlugins.js
@@ -42,12 +42,15 @@ export function makeLicensePlugin(title) {
  * Create a plugin to set the value of the `__SENTRY_DEBUG__` magic string.
  *
  * @param includeDebugging Whether or not the resulting build should include log statements
- * @returns An instance of the `replace` plugin to do the replacement of the magic string with `true` or 'false`
+ * @returns An instance of the `@rollup/plugin-replace` plugin to do the replacement of the magic string with `true` or
+ * 'false`
  */
 export function makeIsDebugBuildPlugin(includeDebugging) {
   return replace({
-    // __DEBUG_BUILD__ and __SENTRY_DEBUG__ are safe to replace in any case, so no checks for assignments necessary
-    preventAssignment: false,
+    // TODO `preventAssignment` will default to true in version 5.x of the replace plugin, at which point we can get rid
+    // of this. (It actually makes no difference in this case whether it's true or false, since we never assign to
+    // `__SENTRY_DEBUG__`, but if we don't give it a value, it will spam with warnings.)
+    preventAssignment: true,
     values: {
       // Flags in current package
       __DEBUG_BUILD__: includeDebugging,
diff --git a/rollup/plugins/npmPlugins.js b/rollup/plugins/npmPlugins.js
diff --git a/scripts/verify-packages-versions.js b/scripts/verify-packages-versions.js

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`module.exports = {`
	`2`	`+ // this is the package root, even when tests are being run at the repo level`
`2`	`3`	`rootDir: process.cwd(),`
`3`	`4`	`collectCoverage: true,`
`4`	`5`	`transform: {`