fix(ci): never fail a job on check_run_id overflow, use the ErrTooManyConfigurations sentinel
- GO-001: bump ermite-actions to b6a9ba7 and match the matrix limit with errors.Is(err, workflow.ErrTooManyConfigurations). - check_run_id is informational: an out-of-range leg, job index, attempt or run id is logged at warn and the id falls back to 0. - ownerRedirect: the owner repository name is parsed once in New, and a failed lookup is logged at debug.
- commit
- c1a72b4b047c4fbe0286e2e7ff5bffe94e13b94c
- author
- brice · · 2026-10-13 00:23 UTC
- parent
- a67d46a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +57 -44
M go.mod +1 -1
| @@ -12,7 +12,7 @@ tool ( | |||
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | 14 | require ( | |
| 15 | - | bnema.dev/ermite-actions v0.0.0-20261012192445-fcc31accb5a4 | |
| 15 | + | bnema.dev/ermite-actions v0.0.0-20261013001922-b6a9ba732350 | |
| 16 | 16 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c | |
| 17 | 17 | bnema.dev/ermite-git v0.1.0 | |
| 18 | 18 | bnema.dev/ermite-kv v0.0.0-20261010031554-5e09b48f81c3 | |
| @@ -12,7 +12,7 @@ tool ( | |||||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | require ( | 14 | require ( | ||
| 15 | - | bnema.dev/ermite-actions v0.0.0-20261012192445-fcc31accb5a4 | 15 | + | bnema.dev/ermite-actions v0.0.0-20261013001922-b6a9ba732350 |
| 16 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c | 16 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c | ||
| 17 | bnema.dev/ermite-git v0.1.0 | 17 | bnema.dev/ermite-git v0.1.0 | ||
| 18 | bnema.dev/ermite-kv v0.0.0-20261010031554-5e09b48f81c3 | 18 | bnema.dev/ermite-kv v0.0.0-20261010031554-5e09b48f81c3 | ||
M go.sum +2 -2
| @@ -1,5 +1,5 @@ | |||
| 1 | - | bnema.dev/ermite-actions v0.0.0-20261012192445-fcc31accb5a4 h1:3TBeF1oKAan41P2AiU1KLGYc4ZTG5KweCe5klMMIkxk= | |
| 2 | - | bnema.dev/ermite-actions v0.0.0-20261012192445-fcc31accb5a4/go.mod h1:qoIB/Iz4xKdLUG4R6wuBDk6cemdugR+UEo5bpSJJYh4= | |
| 1 | + | bnema.dev/ermite-actions v0.0.0-20261013001922-b6a9ba732350 h1:f1DTC/j0kJMfpdN0RBZ586VcG+06m2MSZOZBPDcw0rY= | |
| 2 | + | bnema.dev/ermite-actions v0.0.0-20261013001922-b6a9ba732350/go.mod h1:qoIB/Iz4xKdLUG4R6wuBDk6cemdugR+UEo5bpSJJYh4= | |
| 3 | 3 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c h1:XFaUL8e/4reQ/0M3JUwi/4dmTWspxgwXXg3UQKmFoXQ= | |
| 4 | 4 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c/go.mod h1:L5smjt+IAigyAo3Ny2EnFB9IwvIGOPT1GoByZNgbDa8= | |
| 5 | 5 | bnema.dev/ermite-git v0.1.0 h1:8GNhQDj0MxyvrI4kHewZ9k0rT86e9348T3BJ49WHfmI= | |
| @@ -1,5 +1,5 @@ | |||||
| 1 | - | bnema.dev/ermite-actions v0.0.0-20261012192445-fcc31accb5a4 h1:3TBeF1oKAan41P2AiU1KLGYc4ZTG5KweCe5klMMIkxk= | 1 | + | bnema.dev/ermite-actions v0.0.0-20261013001922-b6a9ba732350 h1:f1DTC/j0kJMfpdN0RBZ586VcG+06m2MSZOZBPDcw0rY= |
| 2 | - | bnema.dev/ermite-actions v0.0.0-20261012192445-fcc31accb5a4/go.mod h1:qoIB/Iz4xKdLUG4R6wuBDk6cemdugR+UEo5bpSJJYh4= | 2 | + | bnema.dev/ermite-actions v0.0.0-20261013001922-b6a9ba732350/go.mod h1:qoIB/Iz4xKdLUG4R6wuBDk6cemdugR+UEo5bpSJJYh4= |
| 3 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c h1:XFaUL8e/4reQ/0M3JUwi/4dmTWspxgwXXg3UQKmFoXQ= | 3 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c h1:XFaUL8e/4reQ/0M3JUwi/4dmTWspxgwXXg3UQKmFoXQ= | ||
| 4 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c/go.mod h1:L5smjt+IAigyAo3Ny2EnFB9IwvIGOPT1GoByZNgbDa8= | 4 | bnema.dev/ermite-api v0.0.0-20261012155218-f8f64413e63c/go.mod h1:L5smjt+IAigyAo3Ny2EnFB9IwvIGOPT1GoByZNgbDa8= | ||
| 5 | bnema.dev/ermite-git v0.1.0 h1:8GNhQDj0MxyvrI4kHewZ9k0rT86e9348T3BJ49WHfmI= | 5 | bnema.dev/ermite-git v0.1.0 h1:8GNhQDj0MxyvrI4kHewZ9k0rT86e9348T3BJ49WHfmI= | ||
M internal/adapters/web/web.go +8 -5
| @@ -105,6 +105,9 @@ type Server struct { | |||
| 105 | 105 | // pathCache holds the path lists the file finder fetches. | |
| 106 | 106 | pathCache *pathCache | |
| 107 | 107 | mux *http.ServeMux | |
| 108 | + | // ownerRepo is the owner login as a repository name, zero when it is not | |
| 109 | + | // a valid one (then no repository can be named like the owner). | |
| 110 | + | ownerRepo domain.RepoName | |
| 108 | 111 | // refsFan shares the fragments of the refs page between the watchers of | |
| 109 | 112 | // a push; entries are few and short-lived. | |
| 110 | 113 | refsFan fanout | |
| @@ -134,6 +137,7 @@ func New(cfg Config) (*Server, error) { | |||
| 134 | 137 | pathCache: newPathCache(pathCacheBytes, pathBuilds), | |
| 135 | 138 | origin: originOf(u), | |
| 136 | 139 | } | |
| 140 | + | s.ownerRepo, _ = domain.ParseRepoName(cfg.Owner) | |
| 137 | 141 | s.mux = s.routes() | |
| 138 | 142 | return s, nil | |
| 139 | 143 | } | |
| @@ -252,12 +256,11 @@ func (s *Server) ownerRedirect(r *http.Request) (string, bool) { | |||
| 252 | 256 | // the owner. A lookup that fails otherwise counts as yes: the request is then | |
| 253 | 257 | // served as it was before, without a redirect. | |
| 254 | 258 | func (s *Server) ownerNamesARepo(r *http.Request) bool { | |
| 255 | - | name, err := domain.ParseRepoName(s.cfg.Owner) | |
| 256 | - | if err != nil { | |
| 257 | - | return false | |
| 259 | + | if s.ownerRepo.IsZero() { | |
| 260 | + | return false // the owner login is not a repository name | |
| 258 | 261 | } | |
| 259 | 262 | ctx := r.Context() | |
| 260 | - | _, err = s.cfg.Repos.Get(ctx, principal(ctx), name) | |
| 263 | + | _, err := s.cfg.Repos.Get(ctx, principal(ctx), s.ownerRepo) | |
| 261 | 264 | switch { | |
| 262 | 265 | case err == nil: | |
| 263 | 266 | return true | |
| @@ -265,7 +268,7 @@ func (s *Server) ownerNamesARepo(r *http.Request) bool { | |||
| 265 | 268 | return false | |
| 266 | 269 | } | |
| 267 | 270 | log := zerowrap.FromCtx(ctx) | |
| 268 | - | log.Warn().Err(err).Msg("owner redirect: the lookup of the repository named like the owner failed") | |
| 271 | + | log.Debug().Err(err).Msg("owner redirect: the lookup of the repository named like the owner failed") | |
| 269 | 272 | return true | |
| 270 | 273 | } | |
| 271 | 274 | ||
| @@ -105,6 +105,9 @@ type Server struct { | |||||
| 105 | // pathCache holds the path lists the file finder fetches. | 105 | // pathCache holds the path lists the file finder fetches. | ||
| 106 | pathCache *pathCache | 106 | pathCache *pathCache | ||
| 107 | mux *http.ServeMux | 107 | mux *http.ServeMux | ||
| 108 | + | // ownerRepo is the owner login as a repository name, zero when it is not | |||
| 109 | + | // a valid one (then no repository can be named like the owner). | |||
| 110 | + | ownerRepo domain.RepoName | |||
| 108 | // refsFan shares the fragments of the refs page between the watchers of | 111 | // refsFan shares the fragments of the refs page between the watchers of | ||
| 109 | // a push; entries are few and short-lived. | 112 | // a push; entries are few and short-lived. | ||
| 110 | refsFan fanout | 113 | refsFan fanout | ||
| @@ -134,6 +137,7 @@ func New(cfg Config) (*Server, error) { | |||||
| 134 | pathCache: newPathCache(pathCacheBytes, pathBuilds), | 137 | pathCache: newPathCache(pathCacheBytes, pathBuilds), | ||
| 135 | origin: originOf(u), | 138 | origin: originOf(u), | ||
| 136 | } | 139 | } | ||
| 140 | + | s.ownerRepo, _ = domain.ParseRepoName(cfg.Owner) | |||
| 137 | s.mux = s.routes() | 141 | s.mux = s.routes() | ||
| 138 | return s, nil | 142 | return s, nil | ||
| 139 | } | 143 | } | ||
| @@ -252,12 +256,11 @@ func (s *Server) ownerRedirect(r *http.Request) (string, bool) { | |||||
| 252 | // the owner. A lookup that fails otherwise counts as yes: the request is then | 256 | // the owner. A lookup that fails otherwise counts as yes: the request is then | ||
| 253 | // served as it was before, without a redirect. | 257 | // served as it was before, without a redirect. | ||
| 254 | func (s *Server) ownerNamesARepo(r *http.Request) bool { | 258 | func (s *Server) ownerNamesARepo(r *http.Request) bool { | ||
| 255 | - | name, err := domain.ParseRepoName(s.cfg.Owner) | 259 | + | if s.ownerRepo.IsZero() { |
| 256 | - | if err != nil { | 260 | + | return false // the owner login is not a repository name |
| 257 | - | return false | |||
| 258 | } | 261 | } | ||
| 259 | ctx := r.Context() | 262 | ctx := r.Context() | ||
| 260 | - | _, err = s.cfg.Repos.Get(ctx, principal(ctx), name) | 263 | + | _, err := s.cfg.Repos.Get(ctx, principal(ctx), s.ownerRepo) |
| 261 | switch { | 264 | switch { | ||
| 262 | case err == nil: | 265 | case err == nil: | ||
| 263 | return true | 266 | return true | ||
| @@ -265,7 +268,7 @@ func (s *Server) ownerNamesARepo(r *http.Request) bool { | |||||
| 265 | return false | 268 | return false | ||
| 266 | } | 269 | } | ||
| 267 | log := zerowrap.FromCtx(ctx) | 270 | log := zerowrap.FromCtx(ctx) | ||
| 268 | - | log.Warn().Err(err).Msg("owner redirect: the lookup of the repository named like the owner failed") | 271 | + | log.Debug().Err(err).Msg("owner redirect: the lookup of the repository named like the owner failed") |
| 269 | return true | 272 | return true | ||
| 270 | } | 273 | } | ||
| 271 | 274 | ||||
M internal/usecase/ci_github.go +21 -20
| @@ -9,13 +9,13 @@ import ( | |||
| 9 | 9 | "encoding/base64" | |
| 10 | 10 | "encoding/binary" | |
| 11 | 11 | "encoding/json" | |
| 12 | - | "errors" | |
| 13 | - | "fmt" | |
| 14 | 12 | "net/url" | |
| 15 | 13 | "strconv" | |
| 16 | 14 | "strings" | |
| 17 | 15 | "time" | |
| 18 | 16 | ||
| 17 | + | "github.com/bnema/zerowrap" | |
| 18 | + | ||
| 19 | 19 | "bnema.dev/ermite/internal/domain" | |
| 20 | 20 | ) | |
| 21 | 21 | ||
| @@ -319,11 +319,14 @@ func (c *CI) githubJSON(ctx context.Context, repo domain.Repo, run domain.CIRun, | |||
| 319 | 319 | // | |
| 320 | 320 | // bits 0-8 combo: matrix leg, 9 bits (0-511); a matrix has at most 256 | |
| 321 | 321 | // bits 9-18 index: position of the job in the workflow file, 10 bits | |
| 322 | - | // (0-1023): a workflow of more jobs than that is refused | |
| 323 | - | // here, ermite has no limit of its own on them | |
| 322 | + | // (0-1023); ermite has no limit of its own on the number | |
| 323 | + | // of jobs | |
| 324 | 324 | // bits 19-25 attempt: run attempt, 7 bits (0-127), so that every re-run | |
| 325 | 325 | // gets new ids, as on GitHub | |
| 326 | 326 | // bits 26-52 run: run id, 27 bits (below 134 million) | |
| 327 | + | // | |
| 328 | + | // A field that overflows would wrap into another instance's id, so the whole | |
| 329 | + | // id is 0 then (see checkRunID). | |
| 327 | 330 | const ( | |
| 328 | 331 | checkComboBits = 9 | |
| 329 | 332 | checkIndexBits = 10 | |
| @@ -331,43 +334,41 @@ const ( | |||
| 331 | 334 | checkRunBits = 53 - checkComboBits - checkIndexBits - checkAttemptBits | |
| 332 | 335 | ) | |
| 333 | 336 | ||
| 334 | - | // errCheckRunID is returned when a job instance does not fit the layout of | |
| 335 | - | // check_run_id: the id is not wrapped or truncated, the job fails with it. | |
| 336 | - | var errCheckRunID = errors.New("the job instance is out of the range of a check run id") | |
| 337 | - | ||
| 338 | 337 | // checkRunID is the number github.com shows in job.check_run_id: derived from | |
| 339 | 338 | // the run, its attempt and the position of the job instance, as ermite's | |
| 340 | 339 | // checks are not numbered. It is unique and stable per attempt, job and matrix | |
| 341 | - | // leg, and fits in a JavaScript number. A value outside its field is an error, | |
| 342 | - | // never a silent wrap that would give two instances the same id. | |
| 343 | - | func checkRunID(run domain.CIRun, index, combo int) (int64, error) { | |
| 340 | + | // leg, and fits in a JavaScript number. | |
| 341 | + | // | |
| 342 | + | // The key is informational, so it never fails a job: a value outside its field | |
| 343 | + | // (which would wrap and give two instances the same id) is logged at warn with | |
| 344 | + | // the run, attempt and job, and the id falls back to 0. | |
| 345 | + | func checkRunID(ctx context.Context, run domain.CIRun, index, combo int) int64 { | |
| 344 | 346 | attempt := int64(run.Attempt) | |
| 345 | 347 | switch { | |
| 346 | 348 | case run.ID < 0 || run.ID >= 1<<checkRunBits, | |
| 347 | 349 | attempt < 0 || attempt >= 1<<checkAttemptBits, | |
| 348 | 350 | index < 0 || index >= 1<<checkIndexBits, | |
| 349 | 351 | combo < 0 || combo >= 1<<checkComboBits: | |
| 350 | - | return 0, fmt.Errorf("%w: run %d, attempt %d, job %d, leg %d", errCheckRunID, run.ID, run.Attempt, index, combo) | |
| 352 | + | log := zerowrap.FromCtx(ctx) | |
| 353 | + | log.Warn().Int64("run_id", run.ID).Int32("attempt", run.Attempt).Int("job_index", index).Int("leg", combo). | |
| 354 | + | Msg("ci check_run_id is out of range: the job context shows 0") | |
| 355 | + | return 0 | |
| 351 | 356 | } | |
| 352 | - | id := run.ID<<(checkAttemptBits+checkIndexBits+checkComboBits) | | |
| 357 | + | return run.ID<<(checkAttemptBits+checkIndexBits+checkComboBits) | | |
| 353 | 358 | attempt<<(checkIndexBits+checkComboBits) | | |
| 354 | 359 | int64(index)<<checkComboBits | | |
| 355 | 360 | int64(combo) | |
| 356 | - | return id, nil | |
| 357 | 361 | } | |
| 358 | 362 | ||
| 359 | 363 | // jobKeys are the keys of the "job" context that the server knows, in | |
| 360 | 364 | // GitHub's order (the runner adds "status"). The runner copies the "job" key | |
| 361 | 365 | // of the contexts it is given into the job context. | |
| 362 | - | func (c *CI) jobKeys(run domain.CIRun, index, combo int) (ordered, error) { | |
| 363 | - | id, err := checkRunID(run, index, combo) | |
| 364 | - | if err != nil { | |
| 365 | - | return nil, err | |
| 366 | - | } | |
| 366 | + | func (c *CI) jobKeys(ctx context.Context, run domain.CIRun, index, combo int) ordered { | |
| 367 | + | id := checkRunID(ctx, run, index, combo) | |
| 367 | 368 | u := c.urls(domain.Repo{Name: run.Repo}) | |
| 368 | 369 | return ordered{ | |
| 369 | 370 | {"check_run_id", id}, | |
| 370 | 371 | {"workflow_ref", u.full + "/" + run.WorkflowPath + "@" + run.Ref}, | |
| 371 | 372 | {"workflow_sha", run.Sha}, {"workflow_repository", u.full}, {"workflow_file_path", run.WorkflowPath}, | |
| 372 | - | }, nil | |
| 373 | + | } | |
| 373 | 374 | } | |
| @@ -9,13 +9,13 @@ import ( | |||||
| 9 | "encoding/base64" | 9 | "encoding/base64" | ||
| 10 | "encoding/binary" | 10 | "encoding/binary" | ||
| 11 | "encoding/json" | 11 | "encoding/json" | ||
| 12 | - | "errors" | |||
| 13 | - | "fmt" | |||
| 14 | "net/url" | 12 | "net/url" | ||
| 15 | "strconv" | 13 | "strconv" | ||
| 16 | "strings" | 14 | "strings" | ||
| 17 | "time" | 15 | "time" | ||
| 18 | 16 | ||||
| 17 | + | "github.com/bnema/zerowrap" | |||
| 18 | + | ||||
| 19 | "bnema.dev/ermite/internal/domain" | 19 | "bnema.dev/ermite/internal/domain" | ||
| 20 | ) | 20 | ) | ||
| 21 | 21 | ||||
| @@ -319,11 +319,14 @@ func (c *CI) githubJSON(ctx context.Context, repo domain.Repo, run domain.CIRun, | |||||
| 319 | // | 319 | // | ||
| 320 | // bits 0-8 combo: matrix leg, 9 bits (0-511); a matrix has at most 256 | 320 | // bits 0-8 combo: matrix leg, 9 bits (0-511); a matrix has at most 256 | ||
| 321 | // bits 9-18 index: position of the job in the workflow file, 10 bits | 321 | // bits 9-18 index: position of the job in the workflow file, 10 bits | ||
| 322 | - | // (0-1023): a workflow of more jobs than that is refused | 322 | + | // (0-1023); ermite has no limit of its own on the number |
| 323 | - | // here, ermite has no limit of its own on them | 323 | + | // of jobs |
| 324 | // bits 19-25 attempt: run attempt, 7 bits (0-127), so that every re-run | 324 | // bits 19-25 attempt: run attempt, 7 bits (0-127), so that every re-run | ||
| 325 | // gets new ids, as on GitHub | 325 | // gets new ids, as on GitHub | ||
| 326 | // bits 26-52 run: run id, 27 bits (below 134 million) | 326 | // bits 26-52 run: run id, 27 bits (below 134 million) | ||
| 327 | + | // | |||
| 328 | + | // A field that overflows would wrap into another instance's id, so the whole | |||
| 329 | + | // id is 0 then (see checkRunID). | |||
| 327 | const ( | 330 | const ( | ||
| 328 | checkComboBits = 9 | 331 | checkComboBits = 9 | ||
| 329 | checkIndexBits = 10 | 332 | checkIndexBits = 10 | ||
| @@ -331,43 +334,41 @@ const ( | |||||
| 331 | checkRunBits = 53 - checkComboBits - checkIndexBits - checkAttemptBits | 334 | checkRunBits = 53 - checkComboBits - checkIndexBits - checkAttemptBits | ||
| 332 | ) | 335 | ) | ||
| 333 | 336 | ||||
| 334 | - | // errCheckRunID is returned when a job instance does not fit the layout of | |||
| 335 | - | // check_run_id: the id is not wrapped or truncated, the job fails with it. | |||
| 336 | - | var errCheckRunID = errors.New("the job instance is out of the range of a check run id") | |||
| 337 | - | ||||
| 338 | // checkRunID is the number github.com shows in job.check_run_id: derived from | 337 | // checkRunID is the number github.com shows in job.check_run_id: derived from | ||
| 339 | // the run, its attempt and the position of the job instance, as ermite's | 338 | // the run, its attempt and the position of the job instance, as ermite's | ||
| 340 | // checks are not numbered. It is unique and stable per attempt, job and matrix | 339 | // checks are not numbered. It is unique and stable per attempt, job and matrix | ||
| 341 | - | // leg, and fits in a JavaScript number. A value outside its field is an error, | 340 | + | // leg, and fits in a JavaScript number. |
| 342 | - | // never a silent wrap that would give two instances the same id. | 341 | + | // |
| 343 | - | func checkRunID(run domain.CIRun, index, combo int) (int64, error) { | 342 | + | // The key is informational, so it never fails a job: a value outside its field |
| 343 | + | // (which would wrap and give two instances the same id) is logged at warn with | |||
| 344 | + | // the run, attempt and job, and the id falls back to 0. | |||
| 345 | + | func checkRunID(ctx context.Context, run domain.CIRun, index, combo int) int64 { | |||
| 344 | attempt := int64(run.Attempt) | 346 | attempt := int64(run.Attempt) | ||
| 345 | switch { | 347 | switch { | ||
| 346 | case run.ID < 0 || run.ID >= 1<<checkRunBits, | 348 | case run.ID < 0 || run.ID >= 1<<checkRunBits, | ||
| 347 | attempt < 0 || attempt >= 1<<checkAttemptBits, | 349 | attempt < 0 || attempt >= 1<<checkAttemptBits, | ||
| 348 | index < 0 || index >= 1<<checkIndexBits, | 350 | index < 0 || index >= 1<<checkIndexBits, | ||
| 349 | combo < 0 || combo >= 1<<checkComboBits: | 351 | combo < 0 || combo >= 1<<checkComboBits: | ||
| 350 | - | return 0, fmt.Errorf("%w: run %d, attempt %d, job %d, leg %d", errCheckRunID, run.ID, run.Attempt, index, combo) | 352 | + | log := zerowrap.FromCtx(ctx) |
| 353 | + | log.Warn().Int64("run_id", run.ID).Int32("attempt", run.Attempt).Int("job_index", index).Int("leg", combo). | |||
| 354 | + | Msg("ci check_run_id is out of range: the job context shows 0") | |||
| 355 | + | return 0 | |||
| 351 | } | 356 | } | ||
| 352 | - | id := run.ID<<(checkAttemptBits+checkIndexBits+checkComboBits) | | 357 | + | return run.ID<<(checkAttemptBits+checkIndexBits+checkComboBits) | |
| 353 | attempt<<(checkIndexBits+checkComboBits) | | 358 | attempt<<(checkIndexBits+checkComboBits) | | ||
| 354 | int64(index)<<checkComboBits | | 359 | int64(index)<<checkComboBits | | ||
| 355 | int64(combo) | 360 | int64(combo) | ||
| 356 | - | return id, nil | |||
| 357 | } | 361 | } | ||
| 358 | 362 | ||||
| 359 | // jobKeys are the keys of the "job" context that the server knows, in | 363 | // jobKeys are the keys of the "job" context that the server knows, in | ||
| 360 | // GitHub's order (the runner adds "status"). The runner copies the "job" key | 364 | // GitHub's order (the runner adds "status"). The runner copies the "job" key | ||
| 361 | // of the contexts it is given into the job context. | 365 | // of the contexts it is given into the job context. | ||
| 362 | - | func (c *CI) jobKeys(run domain.CIRun, index, combo int) (ordered, error) { | 366 | + | func (c *CI) jobKeys(ctx context.Context, run domain.CIRun, index, combo int) ordered { |
| 363 | - | id, err := checkRunID(run, index, combo) | 367 | + | id := checkRunID(ctx, run, index, combo) |
| 364 | - | if err != nil { | |||
| 365 | - | return nil, err | |||
| 366 | - | } | |||
| 367 | u := c.urls(domain.Repo{Name: run.Repo}) | 368 | u := c.urls(domain.Repo{Name: run.Repo}) | ||
| 368 | return ordered{ | 369 | return ordered{ | ||
| 369 | {"check_run_id", id}, | 370 | {"check_run_id", id}, | ||
| 370 | {"workflow_ref", u.full + "/" + run.WorkflowPath + "@" + run.Ref}, | 371 | {"workflow_ref", u.full + "/" + run.WorkflowPath + "@" + run.Ref}, | ||
| 371 | {"workflow_sha", run.Sha}, {"workflow_repository", u.full}, {"workflow_file_path", run.WorkflowPath}, | 372 | {"workflow_sha", run.Sha}, {"workflow_repository", u.full}, {"workflow_file_path", run.WorkflowPath}, | ||
| 372 | - | }, nil | 373 | + | } |
| 373 | } | 374 | } | ||
M internal/usecase/ci_github_test.go +21 -7
| @@ -3,12 +3,15 @@ | |||
| 3 | 3 | package usecase_test | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "bytes" | |
| 6 | 7 | "context" | |
| 7 | 8 | "fmt" | |
| 8 | 9 | "math" | |
| 10 | + | "strconv" | |
| 9 | 11 | "strings" | |
| 10 | 12 | "testing" | |
| 11 | 13 | ||
| 14 | + | "github.com/bnema/zerowrap" | |
| 12 | 15 | "github.com/stretchr/testify/assert" | |
| 13 | 16 | "github.com/stretchr/testify/require" | |
| 14 | 17 | ||
| @@ -57,14 +60,15 @@ func TestRunTitle(t *testing.T) { | |||
| 57 | 60 | // below 2^53, and refuses what does not fit instead of wrapping. | |
| 58 | 61 | func TestCheckRunID(t *testing.T) { | |
| 59 | 62 | t.Parallel() | |
| 63 | + | ctx := context.Background() | |
| 60 | 64 | run := func(id int64, attempt int32) domain.CIRun { return domain.CIRun{ID: id, Attempt: attempt} } | |
| 61 | 65 | seen := map[int64]string{} | |
| 62 | 66 | for _, id := range []int64{1, 2, 1<<27 - 1} { | |
| 63 | 67 | for _, attempt := range []int32{1, 2, 127} { | |
| 64 | 68 | for _, index := range []int{0, 1, 1023} { | |
| 65 | 69 | for _, combo := range []int{0, 1, 255, 511} { | |
| 66 | - | got, err := usecase.CheckRunID(run(id, attempt), index, combo) | |
| 67 | - | require.NoError(t, err) | |
| 70 | + | got := usecase.CheckRunID(ctx, run(id, attempt), index, combo) | |
| 71 | + | require.NotZero(t, got) | |
| 68 | 72 | assert.Less(t, got, int64(1)<<53) | |
| 69 | 73 | key := fmt.Sprintf("run %d attempt %d job %d leg %d", id, attempt, index, combo) | |
| 70 | 74 | assert.NotContains(t, seen, got, "%s collides", key) | |
| @@ -73,12 +77,17 @@ func TestCheckRunID(t *testing.T) { | |||
| 73 | 77 | } | |
| 74 | 78 | } | |
| 75 | 79 | } | |
| 76 | - | a, _ := usecase.CheckRunID(run(7, 1), 0, 0) | |
| 77 | - | b, _ := usecase.CheckRunID(run(7, 2), 0, 0) | |
| 80 | + | a := usecase.CheckRunID(ctx, run(7, 1), 0, 0) | |
| 81 | + | b := usecase.CheckRunID(ctx, run(7, 2), 0, 0) | |
| 78 | 82 | assert.NotEqual(t, a, b, "a re-run gets new ids") | |
| 79 | - | again, _ := usecase.CheckRunID(run(7, 1), 0, 0) | |
| 83 | + | again := usecase.CheckRunID(ctx, run(7, 1), 0, 0) | |
| 80 | 84 | assert.Equal(t, a, again, "stable per attempt") | |
| 81 | 85 | ||
| 86 | + | // In range, the layout is fixed: leg in the low bits, then job, attempt, run. | |
| 87 | + | assert.Equal(t, int64(5)<<26|int64(3)<<19|int64(2)<<9|1, usecase.CheckRunID(ctx, run(5, 3), 2, 1)) | |
| 88 | + | ||
| 89 | + | // Out of range, the id is 0 and the overflow is logged at warn, never an | |
| 90 | + | // error: the key is informational. | |
| 82 | 91 | for name, c := range map[string]struct { | |
| 83 | 92 | run domain.CIRun | |
| 84 | 93 | index, combo int | |
| @@ -91,7 +100,12 @@ func TestCheckRunID(t *testing.T) { | |||
| 91 | 100 | "huge run id": {run(math.MaxInt64, 1), 0, 0}, | |
| 92 | 101 | "negative run": {run(-1, 1), 0, 0}, | |
| 93 | 102 | } { | |
| 94 | - | _, err := usecase.CheckRunID(c.run, c.index, c.combo) | |
| 95 | - | assert.Error(t, err, name) | |
| 103 | + | var logs bytes.Buffer | |
| 104 | + | lctx := zerowrap.WithCtx(ctx, zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs})) | |
| 105 | + | assert.Zero(t, usecase.CheckRunID(lctx, c.run, c.index, c.combo), name) | |
| 106 | + | assert.Contains(t, logs.String(), `"level":"warn"`, name) | |
| 107 | + | assert.Contains(t, logs.String(), `"run_id":`+strconv.FormatInt(c.run.ID, 10), name) | |
| 108 | + | assert.Contains(t, logs.String(), `"attempt":`+strconv.Itoa(int(c.run.Attempt)), name) | |
| 109 | + | assert.Contains(t, logs.String(), `"job_index":`+strconv.Itoa(c.index), name) | |
| 96 | 110 | } | |
| 97 | 111 | } | |
| @@ -3,12 +3,15 @@ | |||||
| 3 | package usecase_test | 3 | package usecase_test | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "bytes" | |||
| 6 | "context" | 7 | "context" | ||
| 7 | "fmt" | 8 | "fmt" | ||
| 8 | "math" | 9 | "math" | ||
| 10 | + | "strconv" | |||
| 9 | "strings" | 11 | "strings" | ||
| 10 | "testing" | 12 | "testing" | ||
| 11 | 13 | ||||
| 14 | + | "github.com/bnema/zerowrap" | |||
| 12 | "github.com/stretchr/testify/assert" | 15 | "github.com/stretchr/testify/assert" | ||
| 13 | "github.com/stretchr/testify/require" | 16 | "github.com/stretchr/testify/require" | ||
| 14 | 17 | ||||
| @@ -57,14 +60,15 @@ func TestRunTitle(t *testing.T) { | |||||
| 57 | // below 2^53, and refuses what does not fit instead of wrapping. | 60 | // below 2^53, and refuses what does not fit instead of wrapping. | ||
| 58 | func TestCheckRunID(t *testing.T) { | 61 | func TestCheckRunID(t *testing.T) { | ||
| 59 | t.Parallel() | 62 | t.Parallel() | ||
| 63 | + | ctx := context.Background() | |||
| 60 | run := func(id int64, attempt int32) domain.CIRun { return domain.CIRun{ID: id, Attempt: attempt} } | 64 | run := func(id int64, attempt int32) domain.CIRun { return domain.CIRun{ID: id, Attempt: attempt} } | ||
| 61 | seen := map[int64]string{} | 65 | seen := map[int64]string{} | ||
| 62 | for _, id := range []int64{1, 2, 1<<27 - 1} { | 66 | for _, id := range []int64{1, 2, 1<<27 - 1} { | ||
| 63 | for _, attempt := range []int32{1, 2, 127} { | 67 | for _, attempt := range []int32{1, 2, 127} { | ||
| 64 | for _, index := range []int{0, 1, 1023} { | 68 | for _, index := range []int{0, 1, 1023} { | ||
| 65 | for _, combo := range []int{0, 1, 255, 511} { | 69 | for _, combo := range []int{0, 1, 255, 511} { | ||
| 66 | - | got, err := usecase.CheckRunID(run(id, attempt), index, combo) | 70 | + | got := usecase.CheckRunID(ctx, run(id, attempt), index, combo) |
| 67 | - | require.NoError(t, err) | 71 | + | require.NotZero(t, got) |
| 68 | assert.Less(t, got, int64(1)<<53) | 72 | assert.Less(t, got, int64(1)<<53) | ||
| 69 | key := fmt.Sprintf("run %d attempt %d job %d leg %d", id, attempt, index, combo) | 73 | key := fmt.Sprintf("run %d attempt %d job %d leg %d", id, attempt, index, combo) | ||
| 70 | assert.NotContains(t, seen, got, "%s collides", key) | 74 | assert.NotContains(t, seen, got, "%s collides", key) | ||
| @@ -73,12 +77,17 @@ func TestCheckRunID(t *testing.T) { | |||||
| 73 | } | 77 | } | ||
| 74 | } | 78 | } | ||
| 75 | } | 79 | } | ||
| 76 | - | a, _ := usecase.CheckRunID(run(7, 1), 0, 0) | 80 | + | a := usecase.CheckRunID(ctx, run(7, 1), 0, 0) |
| 77 | - | b, _ := usecase.CheckRunID(run(7, 2), 0, 0) | 81 | + | b := usecase.CheckRunID(ctx, run(7, 2), 0, 0) |
| 78 | assert.NotEqual(t, a, b, "a re-run gets new ids") | 82 | assert.NotEqual(t, a, b, "a re-run gets new ids") | ||
| 79 | - | again, _ := usecase.CheckRunID(run(7, 1), 0, 0) | 83 | + | again := usecase.CheckRunID(ctx, run(7, 1), 0, 0) |
| 80 | assert.Equal(t, a, again, "stable per attempt") | 84 | assert.Equal(t, a, again, "stable per attempt") | ||
| 81 | 85 | ||||
| 86 | + | // In range, the layout is fixed: leg in the low bits, then job, attempt, run. | |||
| 87 | + | assert.Equal(t, int64(5)<<26|int64(3)<<19|int64(2)<<9|1, usecase.CheckRunID(ctx, run(5, 3), 2, 1)) | |||
| 88 | + | ||||
| 89 | + | // Out of range, the id is 0 and the overflow is logged at warn, never an | |||
| 90 | + | // error: the key is informational. | |||
| 82 | for name, c := range map[string]struct { | 91 | for name, c := range map[string]struct { | ||
| 83 | run domain.CIRun | 92 | run domain.CIRun | ||
| 84 | index, combo int | 93 | index, combo int | ||
| @@ -91,7 +100,12 @@ func TestCheckRunID(t *testing.T) { | |||||
| 91 | "huge run id": {run(math.MaxInt64, 1), 0, 0}, | 100 | "huge run id": {run(math.MaxInt64, 1), 0, 0}, | ||
| 92 | "negative run": {run(-1, 1), 0, 0}, | 101 | "negative run": {run(-1, 1), 0, 0}, | ||
| 93 | } { | 102 | } { | ||
| 94 | - | _, err := usecase.CheckRunID(c.run, c.index, c.combo) | 103 | + | var logs bytes.Buffer |
| 95 | - | assert.Error(t, err, name) | 104 | + | lctx := zerowrap.WithCtx(ctx, zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs})) |
| 105 | + | assert.Zero(t, usecase.CheckRunID(lctx, c.run, c.index, c.combo), name) | |||
| 106 | + | assert.Contains(t, logs.String(), `"level":"warn"`, name) | |||
| 107 | + | assert.Contains(t, logs.String(), `"run_id":`+strconv.FormatInt(c.run.ID, 10), name) | |||
| 108 | + | assert.Contains(t, logs.String(), `"attempt":`+strconv.Itoa(int(c.run.Attempt)), name) | |||
| 109 | + | assert.Contains(t, logs.String(), `"job_index":`+strconv.Itoa(c.index), name) | |||
| 96 | } | 110 | } | ||
| 97 | } | 111 | } | ||
M internal/usecase/ci_plan.go +1 -5
| @@ -449,11 +449,7 @@ func (rs *runState) instance(ctx context.Context, wj *workflow.Job, plan *workfl | |||
| 449 | 449 | if !plan.HasMatrix { | |
| 450 | 450 | matrixCtx = expr.Null{} // GitHub: "matrix": null for a job without a matrix | |
| 451 | 451 | } | |
| 452 | - | jobKeys, err := rs.c.jobKeys(rs.run, base.Index, i) | |
| 453 | - | if err != nil { | |
| 454 | - | return j, err | |
| 455 | - | } | |
| 456 | - | contexts, err := contextsJSON(gh, needs, plan.Context(i), matrixCtx, jobKeys) | |
| 452 | + | contexts, err := contextsJSON(gh, needs, plan.Context(i), matrixCtx, rs.c.jobKeys(ctx, rs.run, base.Index, i)) | |
| 457 | 453 | if err != nil { | |
| 458 | 454 | return j, err | |
| 459 | 455 | } | |
| @@ -449,11 +449,7 @@ func (rs *runState) instance(ctx context.Context, wj *workflow.Job, plan *workfl | |||||
| 449 | if !plan.HasMatrix { | 449 | if !plan.HasMatrix { | ||
| 450 | matrixCtx = expr.Null{} // GitHub: "matrix": null for a job without a matrix | 450 | matrixCtx = expr.Null{} // GitHub: "matrix": null for a job without a matrix | ||
| 451 | } | 451 | } | ||
| 452 | - | jobKeys, err := rs.c.jobKeys(rs.run, base.Index, i) | 452 | + | contexts, err := contextsJSON(gh, needs, plan.Context(i), matrixCtx, rs.c.jobKeys(ctx, rs.run, base.Index, i)) |
| 453 | - | if err != nil { | |||
| 454 | - | return j, err | |||
| 455 | - | } | |||
| 456 | - | contexts, err := contextsJSON(gh, needs, plan.Context(i), matrixCtx, jobKeys) | |||
| 457 | if err != nil { | 453 | if err != nil { | ||
| 458 | return j, err | 454 | return j, err | ||
| 459 | } | 455 | } | ||
M internal/usecase/ci_trigger.go +1 -2
| @@ -495,8 +495,7 @@ func (c *CI) matrixProblem(ctx context.Context, repo domain.Repo, t ciTrigger, f | |||
| 495 | 495 | "github": gh, "needs": expr.NewDictionary(), "inputs": expr.NewDictionary(), "vars": expr.NewDictionary(), | |
| 496 | 496 | }} | |
| 497 | 497 | for _, j := range wf.Jobs { | |
| 498 | - | if _, err := j.EvaluateStrategy(ctx, env); err != nil && strings.Contains(err.Error(), "which exceeds the maximum of") { | |
| 499 | - | // ermite-actions' configurationLimitError is not exported yet. | |
| 498 | + | if _, err := j.EvaluateStrategy(ctx, env); errors.Is(err, workflow.ErrTooManyConfigurations) { | |
| 500 | 499 | return wfPath + ": " + err.Error() | |
| 501 | 500 | } | |
| 502 | 501 | } | |
| @@ -495,8 +495,7 @@ func (c *CI) matrixProblem(ctx context.Context, repo domain.Repo, t ciTrigger, f | |||||
| 495 | "github": gh, "needs": expr.NewDictionary(), "inputs": expr.NewDictionary(), "vars": expr.NewDictionary(), | 495 | "github": gh, "needs": expr.NewDictionary(), "inputs": expr.NewDictionary(), "vars": expr.NewDictionary(), | ||
| 496 | }} | 496 | }} | ||
| 497 | for _, j := range wf.Jobs { | 497 | for _, j := range wf.Jobs { | ||
| 498 | - | if _, err := j.EvaluateStrategy(ctx, env); err != nil && strings.Contains(err.Error(), "which exceeds the maximum of") { | 498 | + | if _, err := j.EvaluateStrategy(ctx, env); errors.Is(err, workflow.ErrTooManyConfigurations) { |
| 499 | - | // ermite-actions' configurationLimitError is not exported yet. | |||
| 500 | return wfPath + ": " + err.Error() | 499 | return wfPath + ": " + err.Error() | ||
| 501 | } | 500 | } | ||
| 502 | } | 501 | } | ||
M internal/usecase/export_test.go +2 -2
| @@ -21,6 +21,6 @@ func (c *CI) RunTitle(ctx context.Context, wf *workflow.Workflow, githubJSON, co | |||
| 21 | 21 | } | |
| 22 | 22 | ||
| 23 | 23 | // CheckRunID exposes checkRunID to the tests. | |
| 24 | - | func CheckRunID(run domain.CIRun, index, combo int) (int64, error) { | |
| 25 | - | return checkRunID(run, index, combo) | |
| 24 | + | func CheckRunID(ctx context.Context, run domain.CIRun, index, combo int) int64 { | |
| 25 | + | return checkRunID(ctx, run, index, combo) | |
| 26 | 26 | } | |
| @@ -21,6 +21,6 @@ func (c *CI) RunTitle(ctx context.Context, wf *workflow.Workflow, githubJSON, co | |||||
| 21 | } | 21 | } | ||
| 22 | 22 | ||||
| 23 | // CheckRunID exposes checkRunID to the tests. | 23 | // CheckRunID exposes checkRunID to the tests. | ||
| 24 | - | func CheckRunID(run domain.CIRun, index, combo int) (int64, error) { | 24 | + | func CheckRunID(ctx context.Context, run domain.CIRun, index, combo int) int64 { |
| 25 | - | return checkRunID(run, index, combo) | 25 | + | return checkRunID(ctx, run, index, combo) |
| 26 | } | 26 | } | ||