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

GitHub status reporter doesn't work #196

Closed
denis-domanskii opened this issue Oct 28, 2020 · 8 comments
Closed

GitHub status reporter doesn't work #196

denis-domanskii opened this issue Oct 28, 2020 · 8 comments
Assignees

Comments

@denis-domanskii
Copy link
Contributor

Summary

I have setup GitHub status hook, but it doesn't work. I don't see status in a GitHub, but see following error in logs:

director_1   | (node:26) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'meta' of undefined
director_1   |     at reportStatusToGithub (/app/dist/lib/hooksReporter.js:87:136)

How to reproduce

Setup and trigger a GitHub hook?

Environment

  • sorry-cypress version: latest
  • cypress version: 5.2.0
  • platform: docker
  • service: director
@jhicken
Copy link
Collaborator

jhicken commented Oct 28, 2020

Any more info you can give would be super helpful.

I have a guess that it has something to do with this line.
https://github.com/sorry-cypress/sorry-cypress/blob/master/packages/director/src/lib/hooksReporter.ts#L100

Which means it got here without the run getting created properly. If this if still happening It would be great to capture the json of the run at this point in time.

I never ran into this but. Its possible that we will need to make this hook not fire if the run has no git sha info.

@denis-domanskii
Copy link
Contributor Author

If this if still happening It would be great to capture the json of the run at this point in time.

I have a stable reproducion of this issue. But i don't know how to capture the json of the run at this point in time.

And how to provide a git sha info to the run? Because my run looks fine, all tests are shown, only hook doesn't work.

@agoldis
Copy link
Collaborator

agoldis commented Oct 28, 2020

@jhicken @denis-domanskii sorry guys, I did introduce this bug recently. it was reported in #193 and should have been fixed in https://github.com/sorry-cypress/sorry-cypress/releases/tag/v1.0.0-beta.9

@denis-domanskii can you please confirm the version you are running

@agoldis agoldis self-assigned this Oct 28, 2020
@denis-domanskii
Copy link
Contributor Author

@agoldis I just pulled fresh docker images and problem has gone. Thank you for the fix.

@jhicken I have faced a new issue when trying to post status. I just got 403 response with data "Cookies must be enabled to use GitHub.". Google helped me to find following isssue: https://github.com/octokit/rest.js/issues/544 , which was resolved because:

I've found that I had to use host: 'api.github.com',. Sorry

Probably we should use this host too? I see that all examples are also uses this URL (https://docs.github.com/en/free-pro-team@latest/rest/reference/repos#statuses)

@jhicken
Copy link
Collaborator

jhicken commented Oct 28, 2020

@denis-domanskii hmmm

So sorry-cypress is not using any special github packages. It simply assembles a url and makes a post with axios. So the code you referenced is maybe a little different.

My testing of this was all done on an enterprise instance of github. So that might be a little different.

That said if all we need to do is add header that shouldn't be a big deal.

We may need to change the url that the hook resource its self is given from the user. Like we may need to change the url field to github api url and add another field for project and another field for repo. I was just trying to make it simple to use. So the user only had to provide 1 piece of information.

I think the first thing to try would be to take the githubDomain from https://github.com/sorry-cypress/sorry-cypress/blob/master/packages/director/src/lib/hooksReporter.ts#L100

And see if we can manipulate it to match the host of the api api.github.com or elsewhere for enterprise use cases. Then test adding host to the headers of the axios request. I'm guessing thats how that github util is using the host attribute. But I may be totally wrong.

Sorry I cant reproduce this. But clearly many people have had this issue with other projects.

Would you be willing to test that theory and add a PR?

@denis-domanskii
Copy link
Contributor Author

denis-domanskii commented Oct 29, 2020

@jhicken I have just checked localy this two URLs (via curl):

  1. https://github.com/api/v3/repos/MyCompany/MyProject/statuses/commitsha123
  2. https://api.github.com/repos/MyCompany/MyProject/statuses/commitsha123

Script (for link 1):

#!/bin/bash
set -e
curl -d '{"state": "success","target_url": "https://example.com","description": "custom description","context": "custom context"}' \
     -H "Authorization: token myGithubToken123" \
      https://github.com/api/v3/repos/MyCompany/MyProject/statuses/commitsha123

Link (1) returned "Cookies must be enabled to use GitHub" response, link (2) was succeed. Both links were used to access an a github.com repo (not an enterprise). I think it can be reproduced with any public repo on this site.

Therefore I propose to create two flows: if ${githubDomain} is a "github.com" then link (2) should be used for request. Else - use current upproach (1). The only case which is not covered - when enterprise version has a separate API endpoint (e.g. api.mygithub.corp). To cover such cases we really need to add new field - API endpoint address.

I'm afraid that it will take to much time to setup environemnt to implement and debug such tiny change. Probably you could implement this fix? If you face any issues with reproducibility - I will be ready to help.

denis-domanskii added a commit to DeniDoman/sorry-cypress that referenced this issue Nov 11, 2020
denis-domanskii added a commit to DeniDoman/sorry-cypress that referenced this issue Nov 11, 2020
denis-domanskii added a commit to DeniDoman/sorry-cypress that referenced this issue Nov 11, 2020
agoldis added a commit that referenced this issue Nov 12, 2020
* Support github.com in a report status publisher (#193)

Endpoint selection logic is added. Also an Accept header is added to the request according to requirements from an API docs.

* #196: unit tests added for the github reporter functionality

* #196: remove accidentally added package-lock.json

* #196: remove one more accidentally added file

* Add typings for projects screen

* Formatting

Co-authored-by: Denis Domanskii <denis.domanskii@jetbrains.com>
Co-authored-by: Andrew Goldis <agoldis@gmail.com>
@agoldis
Copy link
Collaborator

agoldis commented Nov 12, 2020

Resolved by #209

@agoldis agoldis closed this as completed Nov 12, 2020
agoldis added a commit that referenced this issue Dec 20, 2020
* Add kubernetes example deployment

* Add favicon

* v1.0.0-beta.0

* Merge PR feedback, remove failing API readiness probe and clarify the use of k8s secrets

* Commited from github codespace

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

* Update README.md

* Use a non-http mongodb readiness probe

* Fix: screenshots are not shown in TestDetailsV5

`useSwitch` accepts a initialising argument which defaults to false. To be able to display the screenshots on the Test Details we need to provide `true`.

* fix release-dockerhub to accept pre-release tags

* Add semver dependency for gh action

* v1.0.0-beta.1

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

* v1.0.0-beta.2

* Use local time

* v1.0.0-beta.3

* Clarify legal situation

You've created a beautiful project and we're looking to employ your solution as our daily driver soon.
Thank you for all the effort you've put into this.

This PR is meant to clarify the legal situation surrounding the licensing of cypress.
While sorry-cypress may not be legal for open source projects per se, the MIT License is what allows this.

Have a beautiful day

* Update README.md

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

* Fix Date import

* Add "new project" button to welcome screen

* v1.0.0-beta.4

* Use localhost for minio. Resolves #165

* Change issue templates

* Use skip-ci keyword

* [skip-ci] Actually skip

* Use node:12-alpine for dashboard

* Simplify InstanceTestUnion resolver. Closes #172

* v1.0.0-beta.5

* Persist "hide passing tests" in local storage. Closes #139

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

* [skip ci] skip ci and add wsrun

* v1.0.0-beta.6

* Add project search (#186)

* Added filter option for "like" search
Added option to search project by id in dashboard
Added missing api dev dependencies to package.json

* Refactor projects view and code

* Refactor code

* Fix lint errors

* Code review minor fixes

Co-authored-by: Andrew Goldis <agoldis@gmail.com>

* docs: add dlavrenuek as a contributor (#187)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Add run filtering by ci build id (#188)

* Add run filtering by ci build id

* Rename props type name

* Improve code

* Minor code review fixes

Co-authored-by: Andrew Goldis <agoldis@gmail.com>

* v1.0.0-beta.7

* Support grouping (#189)

* Extract route handlers into api module

* Implement grouping withing ciBuildId

* Support gropus for in-memory driver

* More friendly machine names

* Custom CI Link in run details (#182)

* Custom CI Link in run details

* Add few guards to CI_URL

* Remove CI_URL from examples

Co-authored-by: Andrew Goldis <agoldis@gmail.com>

* docs: add Upgreydd as a contributor (#183)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Bump object-path from 0.11.4 to 0.11.5 (#191)

Bumps [object-path](https://github.com/mariocasciaro/object-path) from 0.11.4 to 0.11.5.
- [Release notes](https://github.com/mariocasciaro/object-path/releases)
- [Commits](https://github.com/mariocasciaro/object-path/commits)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* v1.0.0-beta.8

* Remove unneeded port exposure in docker-compose files [skip ci] (#192)

* Remove unneeded port exposure in docker-compose files

* Add dev instructions to CONTRIBUTE.md

Co-authored-by: Andrew Goldis <agoldis@gmail.com>

* Fix packages version [skip ci]

* Fix hooks payload cleanup. Closes #193

* Fix release version and script

* v1.0.0-beta.9

* Fix bug if header don't set. #193

you need to check that there are headers, and then try to parse them.

* Update feature-request.md (#200)

Add reminder to search the issues before posting your feature request.
#199 (comment)

* Add release instructions [skip ci]

* Update release guidelines [skip ci]

* Update FUNDING [skip ci]

* Update README [skip ci]

* Add S3_REGION env to cloudformation (#204)

* rename cloudfront > cloudformation

* Add footer [skip ci]

* use ci-build-id as default group id. closes #203. (#205)

* Use specStats query. Closes #202.

* v1.0.0-beta.10

* Support github.com in a report status publisher (#196) (#209)

* Support github.com in a report status publisher (#193)

Endpoint selection logic is added. Also an Accept header is added to the request according to requirements from an API docs.

* #196: unit tests added for the github reporter functionality

* #196: remove accidentally added package-lock.json

* #196: remove one more accidentally added file

* Add typings for projects screen

* Formatting

Co-authored-by: Denis Domanskii <denis.domanskii@jetbrains.com>
Co-authored-by: Andrew Goldis <agoldis@gmail.com>

* docs: add DeniDoman as a contributor (#210)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Fix director types

* docs: add lucasltinoco as a contributor (#220)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Implement slack hooks. Resolves #120.

Co-authored-by: tim-sendible <tim@sendible.com>
Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Tim Collins <45351296+tico24@users.noreply.github.com>
Co-authored-by: Bram Plessers <webdevotion@gmail.com>
Co-authored-by: Nils Martel <nilsmartel@yahoo.de>
Co-authored-by: Dimitri Lavrenük <dimitri.lavrenuek@camunda.com>
Co-authored-by: dlavrenuek <dimitri.lavr@gmail.com>
Co-authored-by: Kacper <kacper@kacperadler.info>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ivan Pegashev <kj99001@gmail.com>
Co-authored-by: Jeff Hicken <jhicken@gmail.com>
Co-authored-by: Thomas Welton <thomaswelton@me.com>
Co-authored-by: Denis <denidoman@outlook.com>
Co-authored-by: Denis Domanskii <denis.domanskii@jetbrains.com>
@anilpujaraofficial
Copy link

anilpujaraofficial commented Oct 13, 2023

Screenshot from 2023-10-13 12-21-42

how can solve this issue @agoldis

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

No branches or pull requests

4 participants