#6119: improve HTTP Brick labelling/lint rules/network error message

src/analysis/analysisVisitors/httpRequestAnalysis.test.ts

@@ -0,0 +1,165 @@
+/*
+ * Copyright (C) 2023 PixieBrix, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import { type BrickPosition } from "@/bricks/types";
+import { type VisitBlockExtra } from "@/bricks/PipelineVisitor";
+import HttpRequestAnalysis from "@/analysis/analysisVisitors/httpRequestAnalysis";
+import { RemoteMethod } from "@/bricks/transformers/remoteMethod";
+import { AnnotationType } from "@/types/annotationTypes";
+import { makeTemplateExpression } from "@/runtime/expressionCreators";
+
+const position: BrickPosition = {
+  path: "test.path",
+};
+
+describe("RegexAnalysis", () => {
+  test("flags URL parameters as info", () => {
+    const analysis = new HttpRequestAnalysis();
+    analysis.visitBrick(
+      position,
+      {
+        id: RemoteMethod.BLOCK_ID,
+        config: {
+          method: "get",
+          url: "https://example.com/?foo=42",
+        },
+      },
+      {} as VisitBlockExtra
+    );
+
+    expect(analysis.getAnnotations()).toStrictEqual([
+      expect.objectContaining({
+        type: AnnotationType.Info,
+      }),
+    ]);
+  });
+
+  test("flags passing params as data", () => {
+    const analysis = new HttpRequestAnalysis();
+    analysis.visitBrick(
+      position,
+      {
+        id: RemoteMethod.BLOCK_ID,
+        config: {
+          method: "get",
+          url: "https://example.com",
+          data: {
+            foo: 42,
+          },
+        },
+      },
+      {} as VisitBlockExtra
+    );
+
+    expect(analysis.getAnnotations()).toStrictEqual([
+      expect.objectContaining({
+        type: AnnotationType.Warning,
+      }),
+    ]);
+  });
+
+  test("passing string data", () => {
+    const analysis = new HttpRequestAnalysis();
+    analysis.visitBrick(
+      position,
+      {
+        id: RemoteMethod.BLOCK_ID,
+        config: {
+          method: "post",
+          url: "https://example.com",
+          data: makeTemplateExpression(
+            "nunjucks",
+            JSON.stringify({
+              foo: 42,
+            })
+          ),
+        },
+      },
+      {} as VisitBlockExtra
+    );
+
+    expect(analysis.getAnnotations()).toStrictEqual([
+      expect.objectContaining({
+        type: AnnotationType.Warning,
+      }),
+    ]);
+  });
+
+  test("passing data for get", () => {
+    const analysis = new HttpRequestAnalysis();
+    analysis.visitBrick(
+      position,
+      {
+        id: RemoteMethod.BLOCK_ID,
+        config: {
+          method: "get",
+          url: "https://example.com",
+          data: {
+            foo: 42,
+          },
+        },
+      },
+      {} as VisitBlockExtra
+    );
+
+    expect(analysis.getAnnotations()).toStrictEqual([
+      expect.objectContaining({
+        type: AnnotationType.Warning,
+      }),
+    ]);
+  });
+
+  test("valid get request", () => {
+    const analysis = new HttpRequestAnalysis();
+    analysis.visitBrick(
+      position,
+      {
+        id: RemoteMethod.BLOCK_ID,
+        config: {
+          method: "get",
+          url: "https://example.com",
+          params: {
+            foo: 42,
+          },
+        },
+      },
+      {} as VisitBlockExtra
+    );
+
+    expect(analysis.getAnnotations()).toStrictEqual([]);
+  });
+
+  test("valid post request", () => {
+    const analysis = new HttpRequestAnalysis();
+    analysis.visitBrick(
+      position,
+      {
+        id: RemoteMethod.BLOCK_ID,
+        config: {
+          method: "post",
+          url: "https://example.com",
+          data: {
+            foo: 42,
+          },
+        },
+      },
+      {} as VisitBlockExtra
+    );
+
+    expect(analysis.getAnnotations()).toStrictEqual([]);
+  });
+});

src/analysis/analysisVisitors/httpRequestAnalysis.ts

@@ -0,0 +1,143 @@
+/*
+ * Copyright (C) 2023 PixieBrix, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import { AnalysisVisitorABC } from "./baseAnalysisVisitors";
+import { type BrickConfig, type BrickPosition } from "@/bricks/types";
+import { type VisitBlockExtra } from "@/bricks/PipelineVisitor";
+import { joinPathParts } from "@/utils";
+import { AnnotationType } from "@/types/annotationTypes";
+import { castTextLiteralOrThrow } from "@/utils/expressionUtils";
+import { RemoteMethod } from "@/bricks/transformers/remoteMethod";
+import { isEmpty } from "lodash";
+import { type JsonObject } from "type-fest";
+
+function tryParse(value: unknown): JsonObject | null {
+  if (typeof value === "string") {
+    try {
+      // If payload is JSON, parse it for easier reading
+      return JSON.parse(value);
+    } catch {
+      // NOP
+    }
+  }
+
+  return null;
+}
+
+/**
+ * Visitor to detect common mistakes when using the HTTP Request brick.
+ */
+class HttpRequestAnalysis extends AnalysisVisitorABC {
+  get id() {
+    return "http";
+  }
+
+  override visitBrick(
+    position: BrickPosition,
+    blockConfig: BrickConfig,
+    extra: VisitBlockExtra
+  ) {
+    super.visitBrick(position, blockConfig, extra);
+
+    if (blockConfig.id !== RemoteMethod.BLOCK_ID) {
+      return;
+    }
+
+    const { method, data, params, url } = blockConfig.config;
+
+    let urlLiteral;
+    let dataLiteral;
+    let methodLiteral;
+
+    try {
+      urlLiteral = new URL(castTextLiteralOrThrow(url));
+    } catch {
+      // NOP
+    }
+
+    try {
+      methodLiteral = castTextLiteralOrThrow(method);
+    } catch {
+      // NOP
+    }
+
+    try {
+      dataLiteral = tryParse(castTextLiteralOrThrow(data));
+    } catch {
+      // NOP
+    }
+
+    if (methodLiteral === "get" && !isEmpty(data) && isEmpty(params)) {
+      this.annotations.push({
+        position: {
+          path: joinPathParts(position.path, "config", "data"),
+        },
+        message:
+          "Watch Out: APIs typically expect GET request input via URL Search Parameters instead of JSON data.",
+        analysisId: this.id,
+        type: AnnotationType.Warning,
+      });
+    }
+
+    // URLSearchParams returns a symbol in the iterator
+    if (
+      methodLiteral === "get" &&
+      isEmpty(params) &&
+      urlLiteral &&
+      [...urlLiteral.searchParams.keys()].length > 0
+    ) {
+      this.annotations.push({
+        position: {
+          path: joinPathParts(position.path, "config", "url"),
+        },
+        message:
+          "Pro-tip: you can pass URL parameters to the Search Parameters field. When using the Search Parameters field, PixieBrix automatically encodes parameter values.",
+        analysisId: this.id,
+        type: AnnotationType.Info,
+      });
+    }
+
+    if (dataLiteral) {
+      this.annotations.push({
+        position: {
+          path: joinPathParts(position.path, "config", "data"),
+        },
+        message:
+          "Watch Out! You are passing the data as text instead of as an object",
+        analysisId: this.id,
+        type: AnnotationType.Warning,
+      });
+    }
+
+    if (
+      !["get", "delete", "options"].includes(methodLiteral) &&
+      isEmpty(data) &&
+      !isEmpty(params)
+    ) {
+      this.annotations.push({
+        position: {
+          path: joinPathParts(position.path, "config", "params"),
+        },
+        message: `Watch Out! APIs typically expect input to be passed via data payload instead of URL query parameters for ${methodLiteral} requests`,
+        analysisId: this.id,
+        type: AnnotationType.Warning,
+      });
+    }
+  }
+}
+
+export default HttpRequestAnalysis;

src/bricks/transformers/remoteMethod.ts

@@ -27,32 +27,38 @@ import { type SanitizedIntegrationConfig } from "@/types/integrationTypes";
 
 export const inputProperties: Record<string, Schema> = {
   url: {
+    title: "URL",
     type: "string",
-    description: "The API URL",
+    description: "The API endpoint URL",
   },
   service: {
+    title: "Integration Configuration",
     $ref: "https://app.pixiebrix.com/schemas/service#/definitions/configuredService",
     description:
-      "Optional. The service to authenticate the request, if authorization is required",
+      "Optional. The integration configuration to authenticate the request, if authorization is required",
   },
   method: {
+    title: "Method",
     type: "string",
     default: "post",
     description: "The HTTP method",
     enum: ["post", "put", "patch", "delete", "get"],
   },
   params: {
+    title: "Search Parameters",
     type: "object",
     description: "Search/query params",
     additionalProperties: { type: ["string", "number", "boolean"] },
   },
   headers: {
+    title: "Headers",
     type: "object",
     description: "Additional request headers",
     additionalProperties: { type: "string" },
   },
   // Match anything, as valid values are determined by the API being called
   data: {
+    title: "JSON Data",
     description:
       "Supports a JSON payload provided by either a variable or an object",
   },