Uh oh!
There was an error while loading. Please reload this page.
fix(sdk): envvars.update() ReferenceError: name is not defined outside task context - #4431
Conversation
The non-task-context branch of envvars.update() assigned \ = name!, but update()'s implementation signature has no name parameter (it uses nameOrRequestOptions). Calling envvars.update() outside a task run therefore threw ReferenceError: name is not defined. Use nameOrRequestOptions with a type guard, matching the other envvars functions. Closestriggerdotdev#4264
🦋 Changeset detectedLatest commit: 2c8db30 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Hi @karandhaodiyal28-hash, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
| it("throws a clear error when the name is missing", async () => { | ||
| const key = "tr_prod_0123456789abcdefghijklmn"; | ||
| await apiClientManager.runWithConfig({ baseURL: baseUrl, accessToken: key }, async () => { | ||
| await expect( | ||
| // @ts-expect-error deliberately omitting the name argument | ||
| envvars.update("proj_ref", "prod", { value: "abc" }) | ||
| ).rejects.toThrow("name is required"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🔍 Second test will fail: update() throws synchronously and hits the params check first
envvars.update("proj_ref", "prod", { value: "abc" }) maps to projectRefOrName="proj_ref", slugOrParams="prod", nameOrRequestOptions={value:"abc"}, params=undefined. In packages/trigger-sdk/src/v3/envvars.ts:335-337 the if (!params) throw new Error("params is required") check runs before the newly added name check, so the message is "params is required", not "name is required". Moreover the throw is synchronous (the function never returns a promise in this path), so await expect(envvars.update(...)).rejects.toThrow(...) will not catch it — the argument expression throws while being evaluated and the test errors out. This test needs expect(() => envvars.update(...)).toThrow(...) and a case that actually reaches the name check (e.g. update("proj_ref", "prod", undefined as any, { value: "abc" })).
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (typeof nameOrRequestOptions !== "string") { | ||
| throw new Error("name is required"); | ||
| } | ||
| $projectRef = projectRefOrName; | ||
| $slug = slugOrParams; | ||
| $name = name!; | ||
| $name = nameOrRequestOptions; | ||
| $params = params; |
There was a problem hiding this comment.
🔍 Pre-existing mis-assignment in the in-task branch of update()
In the taskContext.ctx branch, $projectRef = slugOrParams and $slug = slugOrParams ?? ctx.environment.slug both take the same argument (packages/trigger-sdk/src/v3/envvars.ts:307-313), and $name falls back to ctx.environment.slug when no name is given. For the 4-arg overload called inside a task this sends the slug as the project ref, so envvars.update(projectRef, slug, name, params) inside a task targets the wrong project. This is outside the diff hunks but is the same argument-routing family of bugs the PR is fixing and would be worth correcting in the same pass (correct form mirrors create/upload: $projectRef = projectRefOrName, $slug = typeof slugOrParams === "string" ? slugOrParams : ctx.environment.slug).
Was this helpful? React with 👍 or 👎 to provide feedback.
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Walkthrough
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closes#4264
Problem
Calling
envvars.update(projectRef, slug, name, params)from outside a task run (a plain Node script / CI deploy step) always throws:create,del,retrieve, andlistall work from the same context — onlyupdateis affected.Root cause
update()'s implementation signature is:There is no
nameparameter (the extraparamsargument shifts the name intonameOrRequestOptions). But the non-task-context branch did:retrieve/dellegitimately have anameparameter, so that line works there — it was carried intoupdatewithout renaming tonameOrRequestOptions. WhentaskContext.ctxis undefined (outside a task), this branch runs and dereferences the out-of-scopename→ReferenceError.Fix
Use
nameOrRequestOptions, guarded like the siblingslug/projectRef/paramschecks in the same branch:This removes the
ReferenceError, correctly routes the env var name, and gives a clear error ifnameis genuinely omitted (instead of a confusing crash).Tests
Added
packages/trigger-sdk/src/v3/envvars.test.ts(modeled onauth.test.ts— local HTTP server +apiClientManager.runWithConfig):update()outside a task context no longer throwsReferenceErrorand dispatches to/projects/{ref}/envvars/{slug}/{name}with the name resolved."name is required".Added a changeset (
@trigger.dev/sdkpatch).Verification note
I verified the overload-resolution logic with a standalone Node reproduction (before →
ReferenceError: name is not defined; after → name resolved correctly; missing name →"name is required"). The full monorepo test suite/build is heavy on my Windows machine, so I couldn't run vitest locally end-to-end — CI will exercise the added test.