Fix UrlParams.makeUrl when globalThis.location is set to `undefined` #2514

rocwang · 2024-04-14T12:04:54Z

Type

Description

Deno has globalThis.location defined as undefined by default. This breaks the HTTP Client in @effect/platform. As the current implemention of baseUrl() as per v0.48.27 only checks "location" in globalThis, but not globalThis.location !== undefined. This PR fixes it by adding the latter.

Here a test script to demostrate the bug:

// filename: test.ts
import { NodeRuntime } from "@effect/platform-node"
import { HttpClient } from "@effect/platform"
import { Console, Effect, pipe } from "effect"

pipe(
  Console.log("location in globalThis:", "location" in globalThis),
  Effect.andThen(Console.log("globalThis.location:", globalThis.location)),
  Effect.as(
    HttpClient.request.get("https://jsonplaceholder.typicode.com/posts/1"),
  ),
  Effect.andThen(HttpClient.client.fetch()),
  Effect.andThen((response) => Console.log("Status:", response.status)),
  Effect.scoped,
  NodeRuntime.runMain,
)

Before the fix:

$ deno run -A ./test.ts
location in globalThis: true
globalThis.location: undefined
timestamp=2024-04-14T11:55:59.777Z level=ERROR fiber=#0 cause="RequestError: InvalidUrl error (GET https://jsonplaceholder.typicode.com/posts/1): Cannot read properties of undefined (reading 'origin')
    at baseUrl (file:///Users/roc/Library/Caches/deno/npm/registry.npmjs.org/@effect/platform/0.48.27/dist/esm/Http/UrlParams.js:98:21)
    at try (file:///Users/roc/Library/Caches/deno/npm/registry.npmjs.org/@effect/platform/0.48.27/dist/esm/Http/UrlParams.js:86:38)
    at file:///Users/roc/Library/Caches/deno/npm/registry.npmjs.org/effect/2.4.18/dist/esm/internal/core-effect.js:46:14"

$ deno run -A --location https://example.com/path ./test.ts
location in globalThis: true
globalThis.location: Location {
  hash: "",
  host: "example.com",
  hostname: "example.com",
  href: "https://example.com/path",
  origin: "https://example.com",
  pathname: "/path",
  port: "",
  protocol: "https:",
  search: ""
}
Status: 200

After the fix:

$ deno run -A ./test.ts                                    
location in globalThis: true
globalThis.location: undefined
Status: 200

$ deno run -A --location https://example.com/path ./test.ts
location in globalThis: true
globalThis.location: Location {
  hash: "",
  host: "example.com",
  hostname: "example.com",
  href: "https://example.com/path",
  origin: "https://example.com",
  pathname: "/path",
  port: "",
  protocol: "https:",
  search: ""
}
Status: 200

A unit test is added for this in the PR.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix UrlParams.makeUrl when globalThis.location is set to `undefined` #2514

Fix UrlParams.makeUrl when globalThis.location is set to `undefined` #2514

rocwang commented Apr 14, 2024 •

edited

Loading

changeset-bot bot commented Apr 14, 2024 •

edited

Loading

tim-smart left a comment

Fix UrlParams.makeUrl when globalThis.location is set to undefined #2514

Fix UrlParams.makeUrl when globalThis.location is set to undefined #2514

Conversation

rocwang commented Apr 14, 2024 • edited Loading

Type

Description

Before the fix:

After the fix:

Related

changeset-bot bot commented Apr 14, 2024 • edited Loading

🦋 Changeset detected

tim-smart left a comment

Choose a reason for hiding this comment

Fix UrlParams.makeUrl when globalThis.location is set to `undefined` #2514

Fix UrlParams.makeUrl when globalThis.location is set to `undefined` #2514

rocwang commented Apr 14, 2024 •

edited

Loading

changeset-bot bot commented Apr 14, 2024 •

edited

Loading