Skip to content

Fix AddonExecutionResult monoid identity#2363

Merged
eikek merged 1 commit intoeikek:masterfrom
v6ak:feature/fix-addon-execution-result-monoid-identity
Nov 7, 2023
Merged

Fix AddonExecutionResult monoid identity#2363
eikek merged 1 commit intoeikek:masterfrom
v6ak:feature/fix-addon-execution-result-monoid-identity

Conversation

@v6ak
Copy link
Copy Markdown
Contributor

@v6ak v6ak commented Nov 7, 2023 •
edited
Loading

When debugging a pure addon that returns invalid JSON, I was getting this error message:

joex 2023.11.07 14:02:43:0000 [io-comp...] [WARN ] docspell.scheduler.impl.SchedulerImpl.wrapTask:283 - Task failed with permanent error
joex java.lang.Exception: Addon execution failed. Do not retry, because some addons were impure.
joex 	at docspell.joex.addon.AddonTaskExtension$AddonExecutionResultOps.raiseErrorIfNeeded(AddonTaskExtension.scala:23)
joex 	at docspell.joex.addon.ItemAddonTask$.$anonfun$apply$4(ItemAddonTask.scala:49)
joex 	at cats.data.OptionT.$anonfun$flatMap$1(OptionT.scala:207)
joex 	at scala.Option.fold(Option.scala:263)
joex 	at cats.data.OptionT.$anonfun$flatMapF$1(OptionT.scala:220)
joex 	at cats.effect.IOFiber.succeeded(IOFiber.scala:1202)
joex 	at cats.effect.IOFiber.runLoop(IOFiber.scala:991)
joex 	at cats.effect.IOFiber.asyncContinueSuccessfulR(IOFiber.scala:1370)
joex 	at cats.effect.IOFiber.run(IOFiber.scala:113)
joex 	at cats.effect.unsafe.WorkerThread.run(WorkerThread.scala:743)
joex

Practical issues

There are two problems:

  • It doesn't write any details about the failure.
  • It considers the addon to be impure, which is not the case. (The addon satisfies !networking && collectOutput.)

Root cause

AddonExecutionResult.empty isn't pure. IIUC, it meets purity requirements, as it cannot cause any side effect. It seems that this value is used just in the Monoid, which causes the impurity to be propagated when the identity element is used somewhere. It also means that the monoid isn't valid.

Fixing

I've adjusted the purity of AddonExecutionResult.empty, and it doesn't break any test. (Well, some LibreOffice-related test is failing in my environment, but that's probably unrelated.) I've also adjusted the tests to cover the monoid.

I still might miss something, but I hope this adjustment is fine.

Error message with the fix

This looks much more descriptive. It is quite clear that some .commands[…].command is missing:

joex java.lang.Exception: Addon execution failed: AddonExecutionResult(List(DecodingError(Attempt to decode value on failed cursor: DownField(command),DownArray,DownField(commands))),true)
joex 	at docspell.joex.addon.AddonTaskExtension$AddonExecutionResultOps.raiseErrorIfNeeded(AddonTaskExtension.scala:18)
joex 	at docspell.joex.addon.ItemAddonTask$.$anonfun$apply$4(ItemAddonTask.scala:49)
joex 	at cats.data.OptionT.$anonfun$flatMap$1(OptionT.scala:207)
joex 	at scala.Option.fold(Option.scala:263)
joex 	at cats.data.OptionT.$anonfun$flatMapF$1(OptionT.scala:220)
joex 	at liftF @ docspell.config.ConfigFactory$.$anonfun$checkSystemProperty$5(ConfigFactory.scala:103)
joex 	at map @ docspell.config.ConfigFactory$.checkSystemProperty(ConfigFactory.scala:97)
joex 	at recover$extension @ fs2.io.file.Files$UnsealedFiles.$anonfun$tempDirectory$1(Files.scala:497)
joex 	at as @ docspell.joex.Main$.$anonfun$run$10(Main.scala:49)
joex 	at map @ docspell.backend.joex.AddonOps$$anon$1.$anonfun$execRunConfig$7(AddonOps.scala:158)
joex 	at modify @ fs2.internal.Scope.close(Scope.scala:262)
joex 	at flatMap @ fs2.Compiler$Target.flatMap(Compiler.scala:163)
joex 	at deferred @ fs2.concurrent.Topic$.apply(Topic.scala:156)
joex 	at offer @ docspell.scheduler.impl.QueueLogger$$anon$1.$anonfun$log$1(QueueLogger.scala:48)
joex 	at delay @ docspell.common.Timestamp$.current(Timestamp.scala:69)
joex 	at map @ docspell.scheduler.impl.LogEvent$.create(LogEvent.scala:46)
joex 	at flatMap @ docspell.scheduler.impl.QueueLogger$$anon$1.log(QueueLogger.scala:42)
joex 	at flatMap @ docspell.backend.joex.AddonOps$$anon$1.$anonfun$execRunConfig$5(AddonOps.scala:157)
joex

@eikek
Copy link
Copy Markdown
Owner

eikek commented Nov 7, 2023

Ah good catch, thanks!!

@eikek eikek added this to the Docspell 0.41.0 milestone Nov 7, 2023
@eikek eikek merged commit f499770 into eikek:master Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Docspell 0.41.0

Development

Successfully merging this pull request may close these issues.

2 participants

@v6ak @eikek