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

Use shared workflows #49

Merged
merged 6 commits into from
Mar 21, 2024
Merged

Use shared workflows #49

merged 6 commits into from
Mar 21, 2024

Conversation

ldemailly
Copy link
Member

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@ldemailly
Copy link
Member Author

ldemailly commented Mar 21, 2024

@aalur @ani1311 @wiardvanrij the tests are failing (even without any of these changes) :

$ go test ./...
--- FAIL: TestApp_singleBurst_Success (0.00s)
    app_test.go:66: Expected processQueue finish the job in ~9 seconds, give or take. Got 7.0958e-05
--- FAIL: TestApp_MultiBurst_Success (0.00s)
    app_test.go:109: Expected processQueue finish the job in ~9 seconds, give or take. Got 3.9959e-05
--- FAIL: TestApp_TestSlackRequestRate (0.00s)
    app_test.go:152: Expected processQueue finish the job in ~5 seconds, give or take. Got 1.7708e-05
{"ts":1710991346.114238,"level":"err","r":54,"file":"main_test.go","line":20,"msg":"Testing logging - err","err":"Channel is not set and Neither attachments, blocks, nor text is set"}
{"ts":1710991346.114729,"level":"err","r":87,"file":"server.go","line":81,"msg":"Invalid request","err":"Channel is not set and Neither attachments, blocks, nor text is set"}
FAIL
FAIL	fortio.org/slack-proxy	2.166s
FAIL

I'll see if I can find a way to either customize the golangci or write a tool to fix all the lll - it's the usual issue when not checking/fixing early, it piles up:

app.go:49: line is 142 characters (lll)
	"team_access_not_granted":                  "The token used is not granted the specific workspace access required to complete this request.",
app.go:50: line is 156 characters (lll)
	"too_many_attachments":                     "Too many attachments were provided with this message. A maximum of 100 attachments are allowed on a message.",
app.go:51: line is 159 characters (lll)
	"too_many_contact_cards":                   "Too many contact_cards were provided with this message. A maximum of 10 contact cards are allowed on a message.",
app.go:57: line is 216 characters (lll)
	"invalid_auth":                             "Some aspect of authentication cannot be validated. Either the provided token is invalid or the request originates from an IP address disallowed from making the request.",
app.go:59: line is 143 characters (lll)
	"missing_scope":                            "The token used is not granted the specific scope permissions required to complete this request.",
app.go:62: line is 242 characters (lll)
	"no_permission":                            "The workspace token used in this request does not have the permissions necessary to complete the request. Make sure your app is a member of the conversation it's attempting to post a message to.",
app.go:63: line is 154 characters (lll)
	"org_login_required":                       "The workspace is undergoing an enterprise migration and will not be available until migration is complete.",
app.go:65: line is 156 characters (lll)
	"token_revoked":                            "Authentication token is for a deleted user or workspace or the app has been removed when using a user token.",
app.go:68: line is 220 characters (lll)
	"fatal_error":                              "The server could not complete your operation(s) without encountering a catastrophic error. It's possible some aspect of the operation succeeded before the error was raised.",
app.go:69: line is 252 characters (lll)
	"internal_error":                           "The server could not complete your operation(s) without encountering an error, likely due to a transient issue on our end. It's possible some aspect of the operation succeeded before the error was raised.",
app.go:70: line is 335 characters (lll)
	"invalid_arg_name":                         "The method was passed an argument whose name falls outside the bounds of accepted or expected values. This includes very long names and names with non-alphanumeric characters other than _. If you get this error, it is typically an indication that you have made a very malformed API call.",
app.go:71: line is 231 characters (lll)
	"invalid_arguments":                        "The method was either called with invalid arguments or some detail about the arguments passed is invalid, which is more likely when using complex arguments like blocks or attachments.",
app.go:73: line is 198 characters (lll)
	"invalid_charset":                          "The method was called via a POST request, but the charset specified in the Content-Type header was invalid. Valid charset names are: utf-8 iso-8859-1.",
app.go:74: line is 227 characters (lll)
	"invalid_form_data":                        "The method was called via a POST request with Content-Type application/x-www-form-urlencoded or multipart/form-data, but the form data was either missing or syntactically invalid.",
app.go:75: line is 233 characters (lll)
	"invalid_post_type":                        "The method was called via a POST request, but the specified Content-Type was invalid. Valid types are: application/json application/x-www-form-urlencoded multipart/form-data text/plain.",
app.go:76: line is 172 characters (lll)
	"missing_post_type":                        "The method was called via a POST request and included a data payload, but the request did not include a Content-Type header.",
app.go:77: line is 144 characters (lll)
	"ratelimited":                              "The request has been ratelimited. Refer to the Retry-After header for when to retry the request.",
app.go:79: line is 262 characters (lll)
	"team_added_to_org":                        "The workspace associated with your request is currently undergoing migration to an Enterprise Organization. Web API and other platform operations will be intermittently unavailable until the transition is complete.",
app.go:83: line is 181 characters (lll)
	"message_limit_exceeded": "Members on this team are sending too many messages. For more details, see https://slack.com/help/articles/115002422943-Usage-limits-for-free-workspaces",
app.go:85: line is 202 characters (lll)
	"fatal_error":            "The server could not complete your operation(s) without encountering a catastrophic error. It's possible some aspect of the operation succeeded before the error was raised.",
app.go:86: line is 234 characters (lll)
	"internal_error":         "The server could not complete your operation(s) without encountering an error, likely due to a transient issue on our end. It's possible some aspect of the operation succeeded before the error was raised.",
app.go:95: line is 154 characters (lll)
	// We are making it a 'soft failure' so that we don't keep retrying it for a period of time for any message that is sent to a channel that doesn't exist.
app.go:127: line is 155 characters (lll)
	// Documentation says that you are allowed the POST the token instead, however that does simply not work. Hence why we are using the Authorization header.
app.go:149: line is 133 characters (lll)
func NewApp(queueSize int, httpClient *http.Client, metrics *Metrics, channelOverride, slackPostMessageURL, slackToken string) *App {
app.go:167: line is 140 characters (lll)
func (app *App) processQueue(ctx context.Context, maxRetries int, initialBackoff time.Duration, burst int, slackRequestRate time.Duration) {
app.go:170: line is 179 characters (lll)
	// Do note that this is best effort, in case of failures, we will exponentially backoff and retry, which will cause the rate to be lower than 1 per second due to obvious reasons.
app.go:176: line is 158 characters (lll)
			// We do check if the channel is closed, but its important to note is that the channel will be closed when the queue is empty and the Shutdown() is called.
app.go:177: line is 141 characters (lll)
			// Simply calling close(app.slackQueue) will not close the channel, it will only prevent any more messages from being sent to the channel.
app.go:184: line is 166 characters (lll)
			// Rate limiter was initially before fetching a message from the queue, but that caused problems by indefinitely looping even if there was no message in the queue.
app.go:188: line is 146 characters (lll)
				log.Fatalf("Error while waiting for rate limiter. This should not happen, provide debug info + error message to an issue if it does: %v", err)
app.go:197: line is 158 characters (lll)
				// Check if the channel is in the doNotProcessChannels map, if it is, check if it's been more than 15 minutes since we last tried to send a message to it.
app.go:200: line is 147 characters (lll)
						// Remove the channel from the map, so that we can process it again. If the channel isn't created in the meantime, we will just add it again.
app.go:224: line is 192 characters (lll)
						log.S(log.Error, "Permanent error, message will not be retried", log.Any("err", err), log.String("description", description), log.String("channel", msg.Channel), log.Any("message", msg))
app.go:229: line is 196 characters (lll)
						log.S(log.Error, "Unknown error, since we can't infer what type of error it is, we will retry it. However, please create a ticket/issue for this project for this error", log.Any("err", err))
app.go:231: line is 189 characters (lll)
					log.S(log.Warning, "Temporary error, message will be retried", log.Any("err", err), log.String("description", description), log.String("channel", msg.Channel), log.Any("message", msg))
app.go:251: line is 157 characters (lll)
			// Need to call this to clean up the wg, which is vital for the shutdown to work (so that we process all the messages in the queue before exiting cleanly)
server_test.go:101: line is 152 characters (lll)
	// If you are running on a non-priviledged account, and get a popup asking for permission to accept incoming connections, you can increase this time...
server_test.go:106: line is 143 characters (lll)
	resp, err := http.Post("http://localhost"+testPort, "application/json", bytes.NewBufferString(`{"channel": "test_channel", "text": "Hello"}`))
server_test.go:122: line is 149 characters (lll)
	secondResp, err := http.Post("http://localhost"+testPort, "application/json", bytes.NewBufferString(`{"channel": "test_channel", "text": "Hello"}`))

@ldemailly
Copy link
Member Author

Started on lll fixer at ldemailly/lll-fixer#1

@ldemailly
Copy link
Member Author

all lll fixed... now about the tests?

@ldemailly
Copy link
Member Author

I think I will merge this and you can fix the tests (and code as it seems config of the rate limiter is broken, possibly because of my request to use duration later in #42 ) - @ani1311

@ldemailly ldemailly mentioned this pull request Mar 21, 2024
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 57.50000% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 46.94%. Comparing base (4819000) to head (0c7d09e).
Report is 9 commits behind head on main.

Files Patch % Lines
app.go 50.00% 15 Missing ⚠️
main.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #49      +/-   ##
==========================================
+ Coverage   46.78%   46.94%   +0.16%     
==========================================
  Files           4        4              
  Lines         404      426      +22     
==========================================
+ Hits          189      200      +11     
- Misses        206      217      +11     
  Partials        9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ldemailly ldemailly merged commit 4ee64b5 into main Mar 21, 2024
7 checks passed
@ldemailly ldemailly deleted the shared_workflows branch March 21, 2024 21:19
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