Repository navigation
Conversation
…ugins is not installed
|
Hi @cocvn157, 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. |
|
Caution Review failedThe pull request is closed.
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. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const importer = vi.fn(async () => { | ||
| const err = Object.assign( | ||
| new Error( | ||
| "Cannot find package '@triggerdotdev/plugins' imported from /triggerdotdev/apps/webapp/build/server/index.js" | ||
| ), | ||
| { code: "ERR_MODULE_NOT_FOUND" } | ||
| ); | ||
| throw err; | ||
| }); | ||
| const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); |
There was a problem hiding this comment.
🟡 New loader tests use forbidden test doubles
The new SSO test uses vi.fn and vi.spyOn instead of exercising a real import. The billing fallback test and RBAC fallback test also add spies, violating the repository's no-mocks rule.
Learn more
The repository's testing rules prohibit mocks, spies, stubs, and fakes, including vi.fn and vi.spyOn. The new SSO case adds both, while the modified billing and RBAC cases add console spies. These cases test the exception classifier with constructed errors rather than observing a real missing-package import.
Example: With the optional package absent, a real dynamic import rejects with Node's own error; these tests instead throw a hand-written string through a fake importer.
Recommended fix: Exercise the real missing-package import in the loader tests. For log assertions, use an integration-level log capture that does not replace or spy on console.error, or restructure the tested behavior to return an observable result.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
| }); | ||
|
|
||
| it("stays silent when Node ESM reports the missing bare package '@triggerdotdev/plugins'", async () => { |
There was a problem hiding this comment.
🟡 New SSO test lacks regression annotation
The new SSO test omits the required // Catches: <bug> comment identifying its distinct regression.
| it("stays silent when Node ESM reports the missing bare package '@triggerdotdev/plugins'", async () => { | |
| // Catches: Missing bare plugin packages incorrectly produce an error-level load failure. | |
| it("stays silent when Node ESM reports the missing bare package '@triggerdotdev/plugins'", async () => { |
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #5083
When
@triggerdotdev/pluginsis not installed (the default state on self-hosted deployments), Node's ESM resolver throwsERR_MODULE_NOT_FOUNDwithCannot find package '@triggerdotdev/plugins' imported from ...rather than quoting the full@triggerdotdev/plugins/<subpath>specifier. Because the RBAC, SSO, and billing loaders only checkedmessage.includes(moduleName), they treated a missing plugin package as a broken plugin and logged"plugin found but failed to load"at error level on boot.This updates the RBAC, SSO, and billing loaders to also match the quoted package name
"'@triggerdotdev/plugins'"when checking whether the plugin itself is absent, keeping missing-plugin fallback silent by default while still logging an error if a transitive dependency of an installed plugin is missing.