chore: add bad reference error [gh-800] #829
Conversation
|
Please fix the test so I can review it |
| @@ -356,8 +356,12 @@ export class ApiCheck extends Check { | |||
|
|
|||
| const deps: ScriptDependency[] = [] | |||
| for (const { filePath, content } of parsed.dependencies) { | |||
| const relativePath = pathToPosix(path.relative(Session.basePath!, filePath)) | |||
| if (relativePath.startsWith('..')) { | |||
| throw new Error(`You cannot reference a dependency file above the Checkly config location folder. Please, modify "${relativePath}" path.`) | |||
There was a problem hiding this comment.
The same error message is used in browser-check.ts. I think that we should move it to a helper file.
| @@ -356,8 +356,12 @@ export class ApiCheck extends Check { | |||
|
|
|||
| const deps: ScriptDependency[] = [] | |||
| for (const { filePath, content } of parsed.dependencies) { | |||
| const relativePath = pathToPosix(path.relative(Session.basePath!, filePath)) | |||
| if (relativePath.startsWith('..')) { | |||
| throw new Error(`You cannot reference a dependency file above the Checkly config location folder. Please, modify "${relativePath}" path.`) | |||
There was a problem hiding this comment.
For the error message, what do you think about:
Failed to parse dependencies for check ${checkLogicalId} "${checkName}". All check dependencies must be contained in the project directory, which is the directory containing the Checkly config file. Checks cannot reference files which are outside of this directory.
Project directory: ${Session.basePath}
Dependency: ${filePath}
I tried to add the check logicalId/name to give a hint where the issue is coming from. I also think logging the project directory could let the user know where they're allowed to put files.
I think that it could make sense to use the absolute path rather than relativePath. I'm not sure if we need to call path.resolve on filePath.
| @@ -111,8 +111,12 @@ export class BrowserCheck extends Check { | |||
|
|
|||
| const deps: CheckDependency[] = [] | |||
| for (const { filePath, content } of parsed.dependencies) { | |||
| const relativePath = pathToPosix(path.relative(Session.basePath!, filePath)) | |||
There was a problem hiding this comment.
What happens if the main browser check script or setup/teardown scripts are outside of the Session.basePath? I'm not sure if checkly-runners supports that, or if we should add an error in that case as well.
|
@umutuzgur @clample can we update this issue? |
|
I'm going to close this one for now |
I hereby confirm that I followed the code guidelines found at engineering guidelines
Affected Components
Notes for the Reviewer
This PRs catches dependency file paths that are above the project CWD and show a user-friendly error.
The check is done for API and Browser checks, because both checks could have not permitted paths for their dependencies.
New Dependency Submission