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

Initial attempt at a generic webhook for reports. #50

Merged
merged 6 commits into from Feb 1, 2022

Conversation

michaelkaye
Copy link
Contributor

Background submission to a notification server that doesn't block rageshake submission.

Timeout requests after 300s (to provide some wiggle room for servers that choose to take action inline)

Add docs on the format of the endpoint (though much of it is pass-through so there is no fixed schema in most cases)

Background submission to a notification server that doesn't block rageshake submission.
@michaelkaye
Copy link
Contributor Author

I'm unsure what the WriteTo error in linting is about at the moment; I haven't adjusted that method or changed calls to it (as far as I'm aware).

@michaelkaye
Copy link
Contributor Author

I'm unsure what the WriteTo error in linting is about at the moment; I haven't adjusted that method or changed calls to it (as far as I'm aware).

Ah, ignore me (ish). Locally, scripts/lint.sh is reporting:

# github.com/matrix-org/rageshake
./submit.go:99:24: method WriteTo(out io.Writer) should have signature WriteTo(io.Writer) (int64, error)

From the go vet incantation

But the actual error in CI is coming from gocyclo complaining that main.go is at 13 now, not sure why go vet isn't reporting/running in CI the same as locally.

@michaelkaye
Copy link
Contributor Author

If 8e00140 is a reasonable pattern, i could replicate for the other config handling sections

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

seems generally plausible

docs/generic_webhook.md Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm otherwise

submit.go Outdated
Comment on lines 525 to 531
// Enrich the parsedPayload with a reportURL and listingURL, to convert a single struct
// to JSON easily
genericHookPayload := genericWebhookPayload{
parsedPayload: p,
ReportURL: reportURL,
ListingURL: listingURL,
}
Copy link
Member

Choose a reason for hiding this comment

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

hoist outside the loop, since it's independent?

(But keep making bytes.Buffer multiple times as these is read from by each http request)
@michaelkaye michaelkaye merged commit a8c57f2 into master Feb 1, 2022
@michaelkaye michaelkaye deleted the michaelk/proxy_rageshake_requests branch February 1, 2022 14:02
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.

None yet

2 participants