Skip to content

Commit 7add442

Browse files
retry spurious compile error, fail matrix job on bad compile (#740)
1 parent d1f804e commit 7add442

6 files changed

Lines changed: 146 additions & 62 deletions

File tree

app/src/App/API.purs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -713,8 +713,8 @@ publish payload = do
713713
let installedResolutions = Path.concat [ tmp, ".registry" ]
714714
buildPlan <- MatrixBuilder.resolutionsToBuildPlan validatedResolutions
715715
MatrixBuilder.installBuildPlan buildPlan installedResolutions
716-
compilationResult <- Run.liftAff $ Purs.callCompiler
717-
{ command: Purs.Compile { globs: [ "src/**/*.purs", Path.concat [ installedResolutions, "*/src/**/*.purs" ] ] }
716+
compilationResult <- Purs.compile
717+
{ globs: [ "src/**/*.purs", Path.concat [ installedResolutions, "*/src/**/*.purs" ] ]
718718
, version: Just compiler
719719
, cwd: Just downloadedPackage
720720
}
@@ -861,8 +861,8 @@ publish payload = do
861861
Just { result: Right _ } -> do
862862
Log.debug "Using cached successful compilation, skipping compilation step"
863863
_ -> do
864-
compilationResult <- Run.liftAff $ Purs.callCompiler
865-
{ command: Purs.Compile { globs: [ Path.concat [ packageSource, "src/**/*.purs" ], Path.concat [ installedResolutions, "*/src/**/*.purs" ] ] }
864+
compilationResult <- Purs.compile
865+
{ globs: [ Path.concat [ packageSource, "src/**/*.purs" ], Path.concat [ installedResolutions, "*/src/**/*.purs" ] ]
866866
, version: Just compiler
867867
, cwd: Just tmp
868868
}
@@ -1018,8 +1018,8 @@ findAllCompilers { source, manifest, compilers } = do
10181018
FS.Extra.ensureDirectory installed
10191019
buildPlanForCompiler <- MatrixBuilder.resolutionsToBuildPlan resolutions
10201020
MatrixBuilder.installBuildPlan buildPlanForCompiler installed
1021-
result <- Run.liftAff $ Purs.callCompiler
1022-
{ command: Purs.Compile { globs: [ Path.concat [ source, "src/**/*.purs" ], Path.concat [ installed, "*/src/**/*.purs" ] ] }
1021+
result <- Purs.compile
1022+
{ globs: [ Path.concat [ source, "src/**/*.purs" ], Path.concat [ installed, "*/src/**/*.purs" ] ]
10231023
, version: Just target
10241024
, cwd: Just workdir
10251025
}

app/src/App/CLI/Purs.purs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@ import Data.Foldable (foldMap)
1212
import Data.String as String
1313
import Node.ChildProcess.Types (Exit(..))
1414
import Node.Library.Execa as Execa
15+
import Registry.App.Effect.Log (LOG)
16+
import Registry.App.Effect.Log as Log
1517
import Registry.Version as Version
18+
import Run (AFF, Run)
19+
import Run as Run
1620

1721
-- | The minimum compiler version that supports 'purs graph'
1822
minPursGraph :: Version
@@ -169,3 +173,34 @@ callCompiler compilerArgs = do
169173
Right ({ errors } :: { errors :: Array CompilerError })
170174
| Array.null errors -> UnknownError "Non-normal exit code, but no errors reported."
171175
| otherwise -> CompilationError errors
176+
177+
-- | The error message pattern indicating a race condition in the PureScript
178+
-- | compiler's parallel compilation. This is a transient error that can be
179+
-- | resolved by retrying.
180+
compilerRaceConditionError :: String
181+
compilerRaceConditionError = "Unexpected end of JSON input"
182+
183+
-- | Check if a compiler failure is due to the known race condition bug.
184+
isCompilerRaceCondition :: CompilerFailure -> Boolean
185+
isCompilerRaceCondition = case _ of
186+
UnknownError err -> String.contains (String.Pattern compilerRaceConditionError) err
187+
_ -> false
188+
189+
-- | Compile PureScript source files with automatic retry for transient errors.
190+
-- | Retries up to 3 times when the "Unexpected end of JSON input" race condition occurs.
191+
compile
192+
:: forall r
193+
. { globs :: Array FilePath, version :: Maybe Version, cwd :: Maybe FilePath }
194+
-> Run (LOG + AFF + r) (Either CompilerFailure String)
195+
compile { globs, version, cwd } = go 1
196+
where
197+
maxRetries = 3
198+
199+
go :: Int -> Run (LOG + AFF + r) (Either CompilerFailure String)
200+
go attempt = do
201+
result <- Run.liftAff $ callCompiler { command: Compile { globs }, version, cwd }
202+
case result of
203+
Left err | isCompilerRaceCondition err && attempt < maxRetries -> do
204+
Log.warn $ "Compiler race condition detected (attempt " <> show attempt <> "/" <> show maxRetries <> "), retrying..."
205+
go (attempt + 1)
206+
_ -> pure result

app/src/App/Effect/PackageSets.purs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,11 @@ handle env = case _ of
234234
compileInstalledPackages :: Version -> Run _ (Either CompilerFailure String)
235235
compileInstalledPackages compiler = do
236236
Log.debug "Compiling installed packages..."
237-
let command = Purs.Compile { globs: [ Path.concat [ Path.basename packagesWorkDir, "**/*.purs" ] ] }
238-
Run.liftAff $ Purs.callCompiler { command, version: Just compiler, cwd: Just env.workdir }
237+
Purs.compile
238+
{ globs: [ Path.concat [ Path.basename packagesWorkDir, "**/*.purs" ] ]
239+
, version: Just compiler
240+
, cwd: Just env.workdir
241+
}
239242

240243
attemptChanges :: Version -> PackageSet -> ChangeSet -> Run _ (Either CompilerFailure PackageSet)
241244
attemptChanges compiler (PackageSet set) changes = do

app/src/App/Server/JobExecutor.purs

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ findNextAvailableJob = runMaybeT
115115
executeJob :: DateTime -> Job -> Run ServerEffects Unit
116116
executeJob _ = case _ of
117117
PublishJob { payload: payload@{ name } } -> do
118+
-- `publish` will throw on error, or return `Nothing` if the pipeline
119+
-- exited with a valid status but without publishing a package (for instance,
120+
-- the package already existed), or return `Just` if a new version was
121+
-- published and we need to queue matrix jobs.
118122
maybeResult <- API.publish payload
119-
-- The above operation will throw if not successful, and return a map of
120-
-- dependencies of the package only if it has not been published before.
121123
for_ maybeResult \{ compiler, dependencies, version } -> do
122124
-- At this point this package has been verified with one compiler only.
123125
-- So we need to enqueue compilation jobs for (1) same package, all the other
@@ -127,45 +129,48 @@ executeJob _ = case _ of
127129
let solverData = { compiler, name, version, dependencies, compilerIndex }
128130
samePackageAllCompilers <- MatrixBuilder.solveForAllCompilers solverData
129131
sameCompilerAllDependants <- MatrixBuilder.solveDependantsForCompiler solverData
130-
for (Array.fromFoldable $ Set.union samePackageAllCompilers sameCompilerAllDependants)
131-
\{ compiler: solvedCompiler, resolutions, name: solvedPackage, version: solvedVersion } -> do
132-
Log.info $ "Enqueuing matrix job: compiler "
133-
<> Version.print solvedCompiler
134-
<> ", package "
135-
<> PackageName.print solvedPackage
136-
<> "@"
137-
<> Version.print solvedVersion
138-
Db.insertMatrixJob
139-
{ payload: resolutions
140-
, compilerVersion: solvedCompiler
141-
, packageName: solvedPackage
142-
, packageVersion: solvedVersion
143-
}
132+
for (Array.fromFoldable $ Set.union samePackageAllCompilers sameCompilerAllDependants) \{ compiler: solvedCompiler, resolutions, name: solvedPackage, version: solvedVersion } -> do
133+
Log.info $ Array.fold
134+
[ "Enqueuing matrix job: compiler "
135+
, Version.print solvedCompiler
136+
, ", package "
137+
, PackageName.print solvedPackage
138+
, "@"
139+
, Version.print solvedVersion
140+
]
141+
Db.insertMatrixJob
142+
{ payload: resolutions
143+
, compilerVersion: solvedCompiler
144+
, packageName: solvedPackage
145+
, packageVersion: solvedVersion
146+
}
144147
UnpublishJob { payload } -> API.authenticated payload
145148
TransferJob { payload } -> API.authenticated payload
146149
MatrixJob details@{ packageName, packageVersion } -> do
147-
maybeDependencies <- MatrixBuilder.runMatrixJob details
148-
-- Unlike the publishing case, after verifying a compilation here we only need
149-
-- to followup with trying to compile the packages that depend on this one
150-
for_ maybeDependencies \dependencies -> do
151-
-- TODO here we are building the compiler index, but we should really cache it
152-
compilerIndex <- MatrixBuilder.readCompilerIndex
153-
let solverData = { compiler: details.compilerVersion, name: packageName, version: packageVersion, dependencies, compilerIndex }
154-
sameCompilerAllDependants <- MatrixBuilder.solveDependantsForCompiler solverData
155-
for (Array.fromFoldable sameCompilerAllDependants)
156-
\{ compiler: solvedCompiler, resolutions, name: solvedPackage, version: solvedVersion } -> do
157-
Log.info $ "Enqueuing matrix job: compiler "
158-
<> Version.print solvedCompiler
159-
<> ", package "
160-
<> PackageName.print solvedPackage
161-
<> "@"
162-
<> Version.print solvedVersion
163-
Db.insertMatrixJob
164-
{ payload: resolutions
165-
, compilerVersion: solvedCompiler
166-
, packageName: solvedPackage
167-
, packageVersion: solvedVersion
168-
}
150+
-- After publishing a matrix job, we check if any dependents need to also have
151+
-- a job queued (for instance a new compiler version came out, we want to have
152+
-- packages trigger jobs for their dependents so it cascades through the registry)
153+
dependencies <- MatrixBuilder.runMatrixJob details
154+
155+
-- TODO here we are building the compiler index, but we should really cache it
156+
compilerIndex <- MatrixBuilder.readCompilerIndex
157+
let solverData = { compiler: details.compilerVersion, name: packageName, version: packageVersion, dependencies, compilerIndex }
158+
sameCompilerAllDependants <- MatrixBuilder.solveDependantsForCompiler solverData
159+
for_ (Array.fromFoldable sameCompilerAllDependants) \{ compiler: solvedCompiler, resolutions, name: solvedPackage, version: solvedVersion } -> do
160+
Log.info $ Array.fold
161+
[ "Enqueuing matrix job: compiler "
162+
, Version.print solvedCompiler
163+
, ", package "
164+
, PackageName.print solvedPackage
165+
, "@"
166+
, Version.print solvedVersion
167+
]
168+
Db.insertMatrixJob
169+
{ payload: resolutions
170+
, compilerVersion: solvedCompiler
171+
, packageName: solvedPackage
172+
, packageVersion: solvedVersion
173+
}
169174
PackageSetJob payload -> API.packageSetUpdate payload
170175

171176
upgradeRegistryToNewCompiler :: forall r. Version -> Run (DB + LOG + EXCEPT String + REGISTRY + r) Unit
@@ -178,12 +183,14 @@ upgradeRegistryToNewCompiler newCompilerVersion = do
178183
-- because from them we should be able to reach the whole of the registry,
179184
-- as they complete new jobs for their dependants will be queued up.
180185
when (Map.isEmpty manifest.dependencies) do
181-
Log.info $ "Enqueuing matrix job for _new_ compiler "
182-
<> Version.print newCompilerVersion
183-
<> ", package "
184-
<> PackageName.print manifest.name
185-
<> "@"
186-
<> Version.print manifest.version
186+
Log.info $ Array.fold
187+
[ "Enqueuing matrix job for _new_ compiler "
188+
, Version.print newCompilerVersion
189+
, ", package "
190+
, PackageName.print manifest.name
191+
, "@"
192+
, Version.print manifest.version
193+
]
187194
void $ Db.insertMatrixJob
188195
{ payload: Map.empty
189196
, compilerVersion: newCompilerVersion

app/src/App/Server/MatrixBuilder.purs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import Run as Run
4646
import Run.Except (EXCEPT)
4747
import Run.Except as Except
4848

49-
runMatrixJob :: forall r. MatrixJobData -> Run (REGISTRY + STORAGE + LOG + AFF + EFFECT + EXCEPT String + r) (Maybe (Map PackageName Range))
49+
runMatrixJob :: forall r. MatrixJobData -> Run (REGISTRY + STORAGE + LOG + AFF + EFFECT + EXCEPT String + r) (Map PackageName Range)
5050
runMatrixJob { compilerVersion, packageName, packageVersion, payload: buildPlan } = do
5151
workdir <- Tmp.mkTmpDir
5252
let installed = Path.concat [ workdir, ".registry" ]
@@ -57,25 +57,27 @@ runMatrixJob { compilerVersion, packageName, packageVersion, payload: buildPlan
5757
(Map.insert packageName packageVersion buildPlan)
5858

5959
installBuildPlan buildPlanWithIntegrity installed
60-
result <- Run.liftAff $ Purs.callCompiler
61-
{ command: Purs.Compile { globs: [ Path.concat [ installed, "*/src/**/*.purs" ] ] }
60+
result <- Purs.compile
61+
{ globs: [ Path.concat [ installed, "*/src/**/*.purs" ] ]
6262
, version: Just compilerVersion
6363
, cwd: Just workdir
6464
}
6565
FS.Extra.remove workdir
6666
case result of
6767
Left err -> do
68-
Log.info $ "Compilation failed with compiler " <> Version.print compilerVersion
69-
<> ":\n"
70-
<> printCompilerFailure compilerVersion err
71-
pure Nothing
68+
Log.info $ Array.fold
69+
[ "Compilation failed with compiler " <> Version.print compilerVersion
70+
, ":\n"
71+
, printCompilerFailure compilerVersion err
72+
]
73+
Except.throw $ "Compilation failed with compiler " <> Version.print compilerVersion
7274
Right _ -> do
7375
Log.info $ "Compilation succeeded with compiler " <> Version.print compilerVersion
7476

7577
Registry.readMetadata packageName >>= case _ of
7678
Nothing -> do
7779
Log.error $ "No existing metadata for " <> PackageName.print packageName
78-
pure Nothing
80+
Except.throw $ "No metadata found for " <> PackageName.print packageName
7981
Just (Metadata metadata) -> do
8082
let
8183
metadataWithCompilers = metadata
@@ -91,10 +93,10 @@ runMatrixJob { compilerVersion, packageName, packageVersion, payload: buildPlan
9193

9294
Log.info "Wrote completed metadata to the registry!"
9395
Registry.readManifest packageName packageVersion >>= case _ of
94-
Just (Manifest manifest) -> pure (Just manifest.dependencies)
96+
Just (Manifest manifest) -> pure manifest.dependencies
9597
Nothing -> do
9698
Log.error $ "No existing metadata for " <> PackageName.print packageName <> "@" <> Version.print packageVersion
97-
pure Nothing
99+
Except.throw $ "No manifest found for " <> PackageName.print packageName <> "@" <> Version.print packageVersion
98100

99101
-- TODO feels like we should be doing this at startup and use the cache instead
100102
-- of reading files all over again

app/test/App/CLI/Purs.purs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import Data.Array as Array
77
import Data.Codec.JSON as CJ
88
import Data.Foldable (traverse_)
99
import Data.Map as Map
10+
import Data.String as String
1011
import JSON as JSON
1112
import Node.FS.Aff as FS.Aff
1213
import Node.Path as Path
@@ -20,6 +21,7 @@ import Registry.Test.Utils (filterAvailableCompilers)
2021
import Registry.Test.Utils as Utils
2122
import Registry.Version as Version
2223
import Test.Spec as Spec
24+
import Test.Spec.Assertions as Assertions
2325

2426
spec :: Spec.Spec Unit
2527
spec = do
@@ -35,6 +37,41 @@ spec = do
3537
traverse_ (testMissingVersion <<< Utils.unsafeVersion) [ "0.13.1", "0.13.7", "0.15.1", "0.12.0", "0.14.12345" ]
3638
traverse_ testCompilationError (filterMaybeVersions [ Just "0.13.0", Just "0.13.8", Just "0.14.0", Just "0.15.0", Nothing ])
3739
traverse_ testGraph (filterMaybeVersions [ Just "0.14.0", Just "0.15.0", Nothing ])
40+
41+
Spec.describe "Compiler Race Condition Detection" do
42+
Spec.it "Detects race condition error in UnknownError" do
43+
let
44+
raceConditionError = UnknownError $ String.joinWith "\n"
45+
[ "[ 1 of 226] Compiling Unsafe.Coerce"
46+
, "[ 2 of 226] Compiling Type.Row"
47+
, "JSON: Unexpected end of JSON input"
48+
]
49+
Purs.isCompilerRaceCondition raceConditionError `Assertions.shouldEqual` true
50+
51+
Spec.it "Detects race condition error with surrounding text" do
52+
let err = UnknownError "Some prefix text\nJSON: Unexpected end of JSON input\nSome suffix"
53+
Purs.isCompilerRaceCondition err `Assertions.shouldEqual` true
54+
55+
Spec.it "Does not detect race condition for normal UnknownError" do
56+
let err = UnknownError "Some other compiler error message"
57+
Purs.isCompilerRaceCondition err `Assertions.shouldEqual` false
58+
59+
Spec.it "Does not detect race condition for CompilationError" do
60+
let
61+
err = CompilationError
62+
[ { position: { startLine: 1, startColumn: 1, endLine: 1, endColumn: 10 }
63+
, message: "Unknown module ModuleB"
64+
, errorCode: "UnknownModule"
65+
, errorLink: "https://example.com"
66+
, filename: "test.purs"
67+
, moduleName: Just "ModuleA"
68+
}
69+
]
70+
Purs.isCompilerRaceCondition err `Assertions.shouldEqual` false
71+
72+
Spec.it "Does not detect race condition for unrelated JSON error" do
73+
let err = UnknownError "JSON parse error: invalid syntax at line 5"
74+
Purs.isCompilerRaceCondition err `Assertions.shouldEqual` false
3875
where
3976
testVersion version =
4077
Spec.it ("Calls compiler version " <> Version.print version) do

0 commit comments

Comments
 (0)