Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: exit on invalid slug #823

Merged

Conversation

drazisil-codecov
Copy link
Contributor

If slug is missing a forward slash, or "undefined", log error and exit

If slug is missing a forward slash, or "undefined", log error and exit
@drazisil-codecov drazisil-codecov requested a review from a team as a code owner July 14, 2022 13:50
@drazisil-codecov drazisil-codecov linked an issue Jul 14, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #823 (04cdb67) into main (c3703d1) will increase coverage by 0.04%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##             main     #823      +/-   ##
==========================================
+ Coverage   92.09%   92.13%   +0.04%     
==========================================
  Files          33       34       +1     
  Lines        1164     1170       +6     
  Branches      237      238       +1     
==========================================
+ Hits         1072     1078       +6     
- Misses         61       62       +1     
+ Partials       31       30       -1     
Flag Coverage Δ
alpine 92.13% <77.77%> (+0.04%) ⬆️
alpine-proxy 92.13% <77.77%> (+0.04%) ⬆️
alpine-without-git 92.13% <77.77%> (+0.04%) ⬆️
linux 92.13% <77.77%> (+0.04%) ⬆️
linux-without-git 92.13% <77.77%> (+0.04%) ⬆️
macos 92.13% <77.77%> (+0.04%) ⬆️
macos-without-git 92.13% <77.77%> (+0.04%) ⬆️
windows 92.13% <77.77%> (+0.04%) ⬆️
windows-without-git 92.13% <77.77%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/index.ts 75.67% <60.00%> (+0.33%) ⬆️
src/helpers/checkSlug.ts 100.00% <100.00%> (ø)

src/index.ts Outdated Show resolved Hide resolved
Comment on lines +1 to +6
export function checkSlug(slug: string | undefined): boolean {
if (slug !== '' && !slug?.match(/\//)) {
return false
}
return true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A future PR should move this function to validate.ts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action is required for this annotation.

@@ -0,0 +1,6 @@
export function checkSlug(slug: string | undefined): boolean {
if (slug !== '' && !slug?.match(/\//)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was extracted from where it existed previously:

if (buildParams.slug !== '' && !buildParams.slug?.match(/\//)) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action is required for this annotation.

@lgtm-com
Copy link

lgtm-com bot commented Jul 25, 2022

This pull request introduces 1 alert when merging 629db8b into 9f1f09b - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Copy link
Contributor Author

@drazisil-codecov drazisil-codecov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix: remove unused import

@lgtm-com
Copy link

lgtm-com bot commented Jul 25, 2022

This pull request introduces 1 alert when merging 5d78f42 into 6e53069 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@mitchell-codecov mitchell-codecov merged commit 510174c into main Jul 25, 2022
@mitchell-codecov mitchell-codecov deleted the 609-branch-flag-is-ignored-when-git-is-unavailable branch July 25, 2022 15:40
This was referenced Aug 1, 2022
logError(`Slug must follow the format of "<owner>/<repo>" or be blank. We detected "${buildParams.slug}"`)
const validSlug = checkSlug(buildParams.slug)
if (!validSlug) {
throw new Error(`Slug must follow the format of "<owner>/<repo>" or be blank. We detected "${buildParams.slug}"`)
Copy link

@sorliem sorliem Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question on this....

I use token authentication to upload codecov reports (using --token flag) and because the slug (using the -r flag) is not being passed through due to the token being present (source), this line is now throwing, and failing my builds to upload the reports to codecov. Is there a way to fix this?

More context: I am using Google Cloud Build, which I noticed is not one of the providers, not sure if that helps.

Logs:

<truncated>
[2022-08-22T17:43:39.633Z] ['info'] Using manual override from args.
[2022-08-22T17:43:39.636Z] ['error'] Error detecting repos setting using git: Error: Unable to detect provider.
[2022-08-22T17:43:39.637Z] ['verbose'] Using the following upload parameters:
[2022-08-22T17:43:39.637Z] ['verbose'] commit
[2022-08-22T17:43:39.637Z] ['verbose'] name
[2022-08-22T17:43:39.637Z] ['verbose'] tag
[2022-08-22T17:43:39.637Z] ['verbose'] flags
[2022-08-22T17:43:39.637Z] ['verbose'] parent
[2022-08-22T17:43:39.637Z] ['error'] There was an error running the uploader: Slug must follow the format of "<owner>/<repo>" or be blank. We detected "undefined"
[2022-08-22T17:43:39.638Z] ['verbose'] The error stack is: Error: Slug must follow the format of "<owner>/<repo>" or be blank. We detected "undefined"
    at main (/snapshot/repo/dist/src/index.js)
[2022-08-22T17:43:39.638Z] ['verbose'] End of uploader: 278 milliseconds

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the excellent investigation, @sorliem. You gave very relevant information and it provided me with a huge head-start on fixing this issue.

slug should not be being validated if a token is passed in.

I created #859 from your comment to track this issue. It should be fixed once #857 is merged.

With regards to Google Cloud Build, I opened #858 to track the feature request.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was referenced Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--branch flag is ignored when git is unavailable
3 participants