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

Fix workflow #3634

Merged
merged 1 commit into from
May 26, 2024
Merged

Fix workflow #3634

merged 1 commit into from
May 26, 2024

Conversation

nvdaes
Copy link
Contributor

@nvdaes nvdaes commented May 25, 2024

No description provided.

@nvdaes
Copy link
Contributor Author

nvdaes commented May 25, 2024

Summary of the issue

In #3294
, a workflow contains a non indentent body.
This bug has been reported by @josephsl and @mltony.
See issue #3633.

@josephsl has detected the exact bug.
Yo u may want to review this @josephsl.

Test

https://github.com/nvdaes/addon-datastore/actions/runs/9237486054

Note: I've exceeded my VirusTotal API key limit today, trying to analyze all add-ons of the store, but the workflow syntax should be fixed.

@nvdaes
Copy link
Contributor Author

nvdaes commented May 25, 2024

I'll submit this to the add-ons mailing list, in case people prefer to wait until this PR is merged.

@josephsl
Copy link
Contributor

Hi,

Thanks for resolving it.

@nvdaes
Copy link
Contributor Author

nvdaes commented May 25, 2024

Thanks @josephsl for your review.

@seanbudd seanbudd merged commit 37fa316 into nvaccess:master May 26, 2024
@seanbudd
Copy link
Member

Thanks @nvdaes @josephsl - I'll re-submit other add-ons monday

@nvdaes
Copy link
Contributor Author

nvdaes commented May 26, 2024

I've submitted clipContentsDesigner 34.0.0 add-on and VirusTotal is not generating a complete json file. Previously this has worked well. This is the same error that I'm finding when I try to analyze all add-ons.
So the PR adding support to VirusTotal may need to be reverted,or you may contact VirusTotal in case there's some issue with their API now.

@nvdaes
Copy link
Contributor Author

nvdaes commented May 26, 2024

@nvdaes
Copy link
Contributor Author

nvdaes commented May 26, 2024

I've seen a recent workflow,and in the VirusTotal job the API key value is blank. NV Access may investigate if the name provided in the workflow for the NV Access secret is right, since in my fork API key is assigned to ***work.
A possible failure maybe related to API key limits too.
BTW, when I want to see my current API key in VirusTotal webpage, the title is VirusTotal - API Key - undefined, and I have exceed ed the daily limit in previous days.

seanbudd added a commit that referenced this pull request May 27, 2024
@seanbudd
Copy link
Member

@nvdaes - can you add logging to catch this error and log the content returned by VT?

SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
    at D:\a\addon-datastore\addon-datastore\.github\workflows\virusTotalAnalysis.js:[19](https://github.com/nvaccess/addon-datastore/actions/runs/9241840563/job/25423917854#step:5:20):25
    at ChildProcess.exithandler (node:child_process:430:5)
    at ChildProcess.emit (node:events:514:28)
    at maybeClose (node:internal/child_process:1105:16)
    at ChildProcess._handle.onexit (node:internal/child_process:305:5)

@nvdaes
Copy link
Contributor Author

nvdaes commented May 27, 2024

can you add logging to catch this error and log the content returned by VT?

Thanks @seanbudd , I'mdoing it, and I expect to create a PR in addon-datastore-staging repo with logging.

@nvdaes
Copy link
Contributor Author

nvdaes commented May 27, 2024

Seems that files aren't found, since in one of my tests I submitted a sendJsonFile without steps to scan the add-on when the issue is created, to be analyzed later.
Really testing this is hard and errors can be produced: I test in the master branch and the PR is created in other branch, so I copy and paste workflows. Perhaps a better strategy for testing is better.

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.

3 participants