Skip to content

Fix noisy runs-on error logging#2102

Merged
mergify[bot] merged 3 commits intonektos:masterfrom
jenseng:fix-runs-on-error-logging
Dec 16, 2023
Merged

Fix noisy runs-on error logging#2102
mergify[bot] merged 3 commits intonektos:masterfrom
jenseng:fix-runs-on-error-logging

Conversation

@jenseng
Copy link
Contributor

@jenseng jenseng commented Nov 23, 2023 •
edited
Loading

Move the logging back up a level to fix a minor logging issue introduced in #2088

The RunContext for a composite action has a dummy/blank Job with no runs-on, meaning its calls to withGithubEnv would result in an inaccurate log message complaining that 'runs-on' key not defined in ...

@jenseng
Fix noisy runs-on error logging
1cd1a0e 
Move the logging back up a level to fix a minor logging issue introduced in nektos#2088

`RunContext`s for composite actions have dummy/blank `Job`s with no `runs-on`,
meaning their calls to `withGithubEnv` would result in an inaccurate log message
complaining that `'runs-on' key not defined in ...`
@jenseng jenseng force-pushed the fix-runs-on-error-logging branch from 96cd6cb to 1cd1a0e Compare November 23, 2023 07:05
@codecov
Copy link

codecov bot commented Nov 23, 2023 •
edited
Loading

Codecov Report

Attention: 872 lines in your changes are missing coverage. Please review.

Comparison is base (4989f44) 61.22% compared to head (676db9a) 61.64%.
Report is 285 commits behind head on master.

Files Patch % Lines
pkg/artifactcache/handler.go 65.46% 102 Missing and 42 partials ⚠️
pkg/runner/run_context.go 73.37% 75 Missing and 19 partials ⚠️
pkg/runner/expression.go 55.17% 66 Missing and 12 partials ⚠️
pkg/runner/action_cache.go 50.74% 49 Missing and 17 partials ⚠️
pkg/container/docker_network.go 0.00% 56 Missing ⚠️
pkg/container/docker_run.go 2.00% 48 Missing and 1 partial ⚠️
pkg/model/workflow.go 43.37% 40 Missing and 7 partials ⚠️
pkg/common/outbound_ip.go 0.00% 44 Missing ⚠️
pkg/container/host_environment.go 0.00% 43 Missing ⚠️
pkg/artifactcache/storage.go 60.22% 24 Missing and 11 partials ⚠️
... and 25 more
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2102      +/-   ##
==========================================
+ Coverage   61.22%   61.64%   +0.42%     
==========================================
  Files          46       53       +7     
  Lines        7141     8808    +1667     
==========================================
+ Hits         4372     5430    +1058     
- Misses       2462     2948     +486     
- Partials      307      430     +123

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

@jenseng jenseng marked this pull request as ready for review November 23, 2023 07:25
@jenseng jenseng requested a review from a team as a code owner November 23, 2023 07:25
@mergify
Copy link
Contributor

mergify bot commented Dec 13, 2023

@jenseng this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Dec 13, 2023
@mergify
Copy link
Contributor

mergify bot commented Dec 16, 2023

@jenseng this pull request has failed checks 🛠

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Dec 16, 2023

Hmm,

fatal error: concurrent map iteration and map write
  
  goroutine 26095 [running]:
  github.com/nektos/act/pkg/runner.valueMasker.func1(0xc0001d6540)
  	/home/runner/work/act/act/pkg/runner/logger.go:154 +0xa9
  github.com/nektos/act/pkg/runner.(*maskedFormatter).Format(0xc0004888d0, 0xc000232f58?)
  	/home/runner/work/act/act/pkg/runner/logger.go:176 +0x64
  github.com/sirupsen/logrus.(*Entry).write(0xc0001d6540)
  	/home/runner/go/pkg/mod/github.com/sirupsen/logrus@v1.9.3/entry.go:289 +0xa5
  github.com/sirupsen/logrus.(*Entry).log(0xc0001da3f0, 0x5, {0xc0009c2a20, 0x23})
  	/home/runner/go/pkg/mod/github.com/sirupsen/logrus@v1.9.3/entry.go:252 +0x474
  github.com/sirupsen/logrus.(*Entry).Log(0xc0001da3f0, 0x5, {0xc0002330f0?, 0x0?, 0x0?})
  	/home/runner/go/pkg/mod/github.com/sirupsen/logrus@v1.9.3/entry.go:304 +0x4f
  github.com/sirupsen/logrus.(*Entry).Logf(0xc0001da3f0, 0x5, {0xd016d2?, 0xcd0fa0?}, {0x0?, 0xa?, 0x40b18d?})
  	/home/runner/go/pkg/mod/github.com/sirupsen/logrus@v1.9.3/entry.go:349 +0x85
  github.com/sirupsen/logrus.(*Entry).Debugf(0xe2e410?, {0xd016d2?, 0xbcca2f?}, {0x0?, 0xc0002331b0?, 0xc0002331b0?})
  	/home/runner/go/pkg/mod/github.com/sirupsen/logrus@v1.9.3/entry.go:358 +0x37
  github.com/nektos/act/pkg/common/git.FindGitRef({0xe2e410, 0xc00032ae40}, {0xc000276c30, 0x2d})
  	/home/runner/work/act/act/pkg/common/git/git.go:90 +0x75
  github.com/nektos/act/pkg/model.(*GithubContext).SetRef(0xc000540960, {0xe2e410, 0xc00032ae40}, {0x0, 0x0}, {0xc000276c30, 0x2d})
  	/home/runner/work/act/act/pkg/model/github_context.go:120 +0x735
  github.com/nektos/act/pkg/runner.(*RunContext).getGithubContext(0xc000a65320, {0xe2e410, 0xc00032ae40})
  	/home/runner/work/act/act/pkg/runner/run_context.go:834 +0xd30
  github.com/nektos/act/pkg/runner.(*stepRun).getGithubContext(0xe2e410?, {0xe2e410?, 0xc00032ae40?})
  	/home/runner/work/act/act/pkg/runner/step_run.go:57 +0xa5
  github.com/nektos/act/pkg/runner.(*RunContext).NewStepExpressionEvaluator(0xc000a65320, {0xe2e410, 0xc00032ae40}, {0xe31270?, 0xc0005bfcc0})
  	/home/runner/work/act/act/pkg/runner/expression.go:134 +0x642
  github.com/nektos/act/pkg/runner.isStepEnabled({0xe2e410, 0xc00032ae40}, {0x0, 0x0}, {0xe31270?, 0xc0005bfcc0?}, 0x1)
  	/home/runner/work/act/act/pkg/runner/step.go:251 +0x125
  github.com/nektos/act/pkg/runner.runStepExecutor.func1({0xe2e410, 0xc00032ae40})
  	/home/runner/work/act/act/pkg/runner/step.go:84 +0x279
  github.com/nektos/act/pkg/runner.newJobExecutor.func4({0xe2e410, 0xc00032ae40})
  	/home/runner/work/act/act/pkg/runner/job_executor.go:78 +0x6a
  github.com/nektos/act/pkg/runner.useStepLogger.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/runner/job_executor.go:197 +0x443
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:148 +0x144
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:136 +0x34
  github.com/nektos/act/pkg/common.Executor.Finally.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:182 +0x34
  github.com/nektos/act/pkg/common.Executor.Finally.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:182 +0x34
  github.com/nektos/act/pkg/common.Executor.Finally.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:182 +0x34
  github.com/nektos/act/pkg/common.Executor.Then.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/common/executor.go:148 +0x144
  github.com/nektos/act/pkg/runner.(*RunContext).Executor.func1({0xe2e410, 0xc0004bfe90})
  	/home/runner/work/act/act/pkg/runner/run_context.go:620 +0x8b
  github.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1.1({0xe2e410, 0xc00052f9b0})
  	/home/runner/work/act/act/pkg/runner/runner.go:192 +0x1e3
  github.com/nektos/act/pkg/common.NewParallelExecutor.func1.1(0x803a26?, 0xc000a9a000?)
  	/home/runner/work/act/act/pkg/common/executor.go:107 +0x5c
  created by github.com/nektos/act/pkg/common.NewParallelExecutor.func1
  	/home/runner/work/act/act/pkg/common/executor.go:105 +0x10b

weird, never saw this crash in the tests.

@mergify mergify bot removed the needs-work Extra attention is needed label Dec 16, 2023
@mergify mergify bot merged commit 00fbfa7 into nektos:master Dec 16, 2023
jmikedupont2 pushed a commit to meta-introspector/act that referenced this pull request Mar 10, 2024
@jenseng @wolfogre @mergify
Fix noisy runs-on error logging (nektos#2102)
7000822 
Move the logging back up a level to fix a minor logging issue introduced in nektos#2088

`RunContext`s for composite actions have dummy/blank `Job`s with no `runs-on`,
meaning their calls to `withGithubEnv` would result in an inaccurate log message
complaining that `'runs-on' key not defined in ...`

Co-authored-by: Jason Song <i@wolfogre.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wolfogre wolfogre wolfogre approved these changes

@ChristopherHX ChristopherHX ChristopherHX approved these changes

Assignees

No one assigned

Labels

size/XS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jenseng @ChristopherHX @wolfogre