Skip to content

Commit

Permalink
LF: Add context in missing package errors (#10418)
Browse files Browse the repository at this point in the history
explaining the origining of missing package.

This is part of #9974

CHANGELOG_BEGIN
CHANGELOG_END
  • Loading branch information
remyhaemmerle-da authored Jul 28, 2021
1 parent 41009f7 commit 73290c2
Show file tree
Hide file tree
Showing 13 changed files with 85 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import com.daml.lf.transaction.Node._
import com.daml.lf.value.Value
import java.nio.file.Files

import com.daml.lf.language.{Interface, LanguageVersion, StablePackages}
import com.daml.lf.language.{Interface, LanguageVersion, LookupError, StablePackages}
import com.daml.lf.validation.Validation
import com.daml.lf.value.Value.ContractId
import com.daml.nameof.NameOf
Expand Down Expand Up @@ -230,21 +230,16 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
} yield validationResult
}

private[engine] def loadPackages(pkgIds: List[PackageId]): Result[Unit] =
pkgIds.dropWhile(compiledPackages.packageIds.contains) match {
case pkgId :: rest =>
ResultNeedPackage(
pkgId,
{
case Some(pkg) =>
compiledPackages.addPackage(pkgId, pkg).flatMap(_ => loadPackages(rest))
case None =>
ResultError(Error.Package.MissingPackage(pkgId))
},
)
case Nil =>
ResultDone.Unit
}
private[engine] def loadPackage(pkgId: PackageId, context: language.Reference): Result[Unit] =
ResultNeedPackage(
pkgId,
{
case Some(pkg) =>
compiledPackages.addPackage(pkgId, pkg)
case None =>
ResultError(Error.Package.MissingPackage(pkgId, context))
},
)

@inline
private[lf] def runSafely[X](
Expand All @@ -256,9 +251,12 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
} catch {
// The two following error should be prevented by the type checking does by translateCommand
// so it’s an internal error.
case error: speedy.Compiler.PackageNotFound =>
case speedy.Compiler.PackageNotFound(pkgId, context) =>
ResultError(
Error.Preprocessing.Internal(funcName, s"CompilationError: ${error.getMessage}")
Error.Preprocessing.Internal(
funcName,
s"CompilationError: " + LookupError.MissingPackage.pretty(pkgId, context),
)
)
case speedy.Compiler.CompilationError(error) =>
ResultError(Error.Preprocessing.Internal(funcName, s"CompilationError: $error"))
Expand Down Expand Up @@ -349,9 +347,10 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
case SError.SErrorDamlException(error) =>
Error.Interpretation.DamlException(error)
}
case SResultNeedPackage(pkgId, callback) =>
case SResultNeedPackage(pkgId, context, callback) =>
return Result.needPackage(
pkgId,
context,
pkg => {
compiledPackages.addPackage(pkgId, pkg).flatMap { _ =>
callback(compiledPackages)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package com.daml.lf
package engine

import com.daml.lf.data.Ref
import com.daml.lf.language.Ast
import com.daml.lf.language.{Ast, LookupError}
import com.daml.lf.transaction.NodeId
import com.daml.lf.value.Value

Expand Down Expand Up @@ -35,12 +35,14 @@ object Error {
def msg: String = validationError.pretty
}

final case class MissingPackages(packageIds: Set[Ref.PackageId]) extends Error {
val s = if (packageIds.size <= 1) "" else "s"
override def msg: String = s"Couldn't find package$s ${packageIds.mkString(",")}"
final case class MissingPackage(packageId: Ref.PackageId, context: language.Reference)
extends Error {
override def msg: String = LookupError.MissingPackage.pretty(packageId, context)
}
private[engine] object MissingPackage {
def apply(packageId: Ref.PackageId): MissingPackages = MissingPackages(Set(packageId))

object MissingPackage {
def apply(packageId: Ref.PackageId): MissingPackage =
MissingPackage(packageId, language.Reference.Package(packageId))
}

final case class AllowedLanguageVersion(
Expand Down Expand Up @@ -90,14 +92,6 @@ object Error {
override def msg: String = lookupError.pretty
}

private[engine] object MissingPackage {
def unapply(error: Lookup): Option[Ref.PackageId] =
error.lookupError match {
case language.LookupError(language.Reference.Package(packageId), _) => Some(packageId)
case _ => None
}
}

final case class TypeMismatch(
typ: Ast.Type,
value: Value[Value.ContractId],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,16 @@ final case class ResultNeedKey[A](

object Result {
// fails with ResultError if the package is not found
private[lf] def needPackage[A](packageId: PackageId, resume: Package => Result[A]) =
private[lf] def needPackage[A](
packageId: PackageId,
context: language.Reference,
resume: Package => Result[A],
) =
ResultNeedPackage(
packageId,
{
case Some(pkg) => resume(pkg)
case None => ResultError(Error.Package.MissingPackage(packageId))
case None => ResultError(Error.Package.MissingPackage(packageId, context))
},
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ final class ValueEnricher(engine: Engine) {

private[this] def handleLookup[X](lookup: => Either[LookupError, X]) = lookup match {
case Right(value) => ResultDone(value)
case Left(LookupError.MissingPackage(pkgId)) =>
case Left(LookupError.MissingPackage(pkgId, context)) =>
engine
.loadPackages(List(pkgId))
.loadPackage(pkgId, context)
.flatMap(_ =>
lookup match {
case Right(value) => ResultDone(value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private[engine] final class Preprocessor(compiledPackages: MutableCompiledPackag
tyConAlreadySeen0: Set[Ref.TypeConName],
tmplsAlreadySeen0: Set[Ref.TypeConName],
): Result[(Set[Ref.TypeConName], Set[Ref.TypeConName])] = {
def pullPackage(pkgId: Ref.PackageId) =
def pullPackage(pkgId: Ref.PackageId, context: language.Reference) =
ResultNeedPackage(
pkgId,
{
Expand All @@ -55,7 +55,7 @@ private[engine] final class Preprocessor(compiledPackages: MutableCompiledPackag
)
} yield r
case None =>
ResultError(Error.Package.MissingPackage(pkgId))
ResultError(Error.Package.MissingPackage(pkgId, context))
},
)

Expand All @@ -81,8 +81,8 @@ private[engine] final class Preprocessor(compiledPackages: MutableCompiledPackag
tyConAlreadySeen0 + tyCon,
tmplsAlreadySeen0,
)
case Left(LookupError.MissingPackage(pkgId)) =>
pullPackage(pkgId)
case Left(LookupError.MissingPackage(pkgId, context)) =>
pullPackage(pkgId, context)
case Left(e) =>
ResultError(Error.Preprocessing.Lookup(e))
}
Expand All @@ -107,8 +107,8 @@ private[engine] final class Preprocessor(compiledPackages: MutableCompiledPackag
if (tyConAlreadySeen0(tmplId)) typs0 else Ast.TTyCon(tmplId) :: typs0
val typs2 = template.key.fold(typs1)(_.typ :: typs1)
go(typs2, tmplsToProcess, tyConAlreadySeen0, tmplsAlreadySeen0)
case Left(LookupError.MissingPackage(pkgId)) =>
pullPackage(pkgId)
case Left(LookupError.MissingPackage(pkgId, context)) =>
pullPackage(pkgId, context)
case Left(error) =>
ResultError(Error.Preprocessing.Lookup(error))
}
Expand Down Expand Up @@ -180,7 +180,7 @@ private[preprocessing] object Preprocessor {
try {
ResultDone(unsafeRun)
} catch {
case Error.Preprocessing.MissingPackage(_) =>
case Error.Preprocessing.Lookup(LookupError.MissingPackage(_, _)) =>
handleMissingPackages.flatMap(_ => start)
case e: Error.Preprocessing.Error =>
ResultError(e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,13 @@ private[lf] object Compiler {
languageVersion: language.LanguageVersion,
allowedLanguageVersions: VersionRange[language.LanguageVersion],
) extends RuntimeException(s"Disallowed language version $languageVersion", null, true, false)
case class PackageNotFound(pkgId: PackageId)
extends RuntimeException(s"Package not found $pkgId", null, true, false)
case class PackageNotFound(pkgId: PackageId, context: language.Reference)
extends RuntimeException(
language.LookupError.MissingPackage.pretty(pkgId, context),
null,
true,
false,
)

// NOTE(MH): We make this an enum type to avoid boolean blindness. In fact,
// other profiling modes like "only trace the ledger interactions" might also
Expand Down Expand Up @@ -111,7 +116,8 @@ private[lf] object Compiler {
})
} catch {
case CompilationError(msg) => Left(s"Compilation Error: $msg")
case PackageNotFound(pkgId) => Left(s"Package not found $pkgId")
case PackageNotFound(pkgId, context) =>
Left(LookupError.MissingPackage.pretty(pkgId, context))
case e: ValidationError => Left(e.pretty)
}
}
Expand Down Expand Up @@ -357,9 +363,9 @@ private[lf] final class Compiler(
case Compiler.NoPackageValidation =>
case Compiler.FullPackageValidation =>
Validation.checkPackage(interface, pkgId, pkg).left.foreach {
case EUnknownDefinition(_, LookupError.MissingPackage(pkgId_)) =>
case EUnknownDefinition(_, LookupError.MissingPackage(pkgId_, context)) =>
logger.trace(s"compilePackage: Missing $pkgId_, requesting it...")
throw PackageNotFound(pkgId_)
throw PackageNotFound(pkgId_, context)
case e =>
throw e
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1398,25 +1398,14 @@ private[lf] object SBuiltin {

/** $any-exception-message :: AnyException -> Text */
final case object SBAnyExceptionMessage extends SBuiltin(1) {
private def exceptionMessage(tyCon: TypeConName, value: SValue, machine: Machine) =
machine.ctrl = SEApp(SEVal(ExceptionMessageDefRef(tyCon)), Array(SEValue(value)))

override private[speedy] def execute(args: util.ArrayList[SValue], machine: Machine): Unit = {
val exception = getSAnyException(args, 0)
val tyCon = exception.id
if (tyCon == ValueArithmeticError.tyCon)
machine.returnValue = exception.values.get(0)
else if (!machine.compiledPackages.packageIds.contains(exception.id.packageId))
throw SpeedyHungry(
SResultNeedPackage(
tyCon.packageId,
{ packages =>
machine.compiledPackages = packages
exceptionMessage(tyCon, exception, machine)
},
)
)
exceptionMessage(tyCon, exception, machine)
exception.id match {
case ValueArithmeticError.tyCon =>
machine.returnValue = exception.values.get(0)
case tyCon =>
machine.ctrl = SEApp(SEVal(ExceptionMessageDefRef(tyCon)), Array(SEValue(exception)))
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

package com.daml.lf.speedy
package com.daml.lf
package speedy

import com.daml.lf.CompiledPackages
import com.daml.lf.value.Value.{ContractId, ContractInst}
import com.daml.lf.data.Ref._
import com.daml.lf.data.Time
Expand Down Expand Up @@ -45,6 +45,7 @@ object SResult {
*/
final case class SResultNeedPackage(
pkg: PackageId,
context: language.Reference,
callback: CompiledPackages => Unit,
) extends SResult

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,7 @@ private[lf] object Speedy {
throw SpeedyHungry(
SResultNeedPackage(
ref.packageId,
language.Reference.Package(ref.packageId),
{ packages =>
this.compiledPackages = packages
// To avoid infinite loop in case the packages are not updated properly by the caller
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class InterpreterTest extends AnyWordSpec with Matchers with TableDrivenProperty
val machine = Speedy.Machine.fromPureExpr(pkgs1, EVal(ref))
val result = machine.run()
result match {
case SResultNeedPackage(pkgId, cb) =>
case SResultNeedPackage(pkgId, _, cb) =>
ref.packageId shouldBe pkgId
cb(pkgs2)
val result = machine.run()
Expand All @@ -246,7 +246,7 @@ class InterpreterTest extends AnyWordSpec with Matchers with TableDrivenProperty
val machine = Speedy.Machine.fromPureExpr(pkgs1, EVal(ref))
val result = machine.run()
result match {
case SResultNeedPackage(pkgId, cb) =>
case SResultNeedPackage(pkgId, _, cb) =>
ref.packageId shouldBe pkgId
try {
cb(pkgs3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,7 @@ object SBuiltinTest {
val value = machine.run() match {
case SResultFinalValue(v) => v
case SResultError(err) => throw Goodbye(err)
case SResultNeedPackage(pkgId, _) =>
case SResultNeedPackage(pkgId, _, _) =>
throw Goodbye(SErrorCrash("", s"need package '$pkgId'"))
case res => throw new RuntimeException(s"Got unexpected interpretation result $res")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,29 @@ import com.daml.lf.data.Ref._

final case class LookupError(notFound: Reference, context: Reference) {
val pretty: String = "unknown " + notFound.pretty + (
if (context == notFound) "" else " while looking for " + context.pretty
if (context == notFound) "" else LookupError.contextDetails(context)
)
}

object LookupError {

def contextDetails(context: Reference): String =
context match {
case Reference.Package(_) => ""
case otherwise => " while looking for " + otherwise.pretty
}

object MissingPackage {
def unapply(err: LookupError): Option[PackageId] =
def unapply(err: LookupError): Option[(PackageId, Reference)] =
err.notFound match {
case Reference.Package(packageId) => Some(packageId)
case Reference.Package(packageId) => Some(packageId -> err.context)
case _ => None
}

def pretty(pkgId: PackageId, context: Reference): String =
s"Couldn't find package $pkgId" + contextDetails(context)
}

}

sealed abstract class Reference extends Product with Serializable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ package scenario
import com.daml.lf.data.Ref._
import com.daml.lf.data.{ImmArray, Ref, Time}
import com.daml.lf.engine.Engine
import com.daml.lf.language.Ast
import com.daml.lf.transaction.{NodeId, GlobalKey, SubmittedTransaction}
import com.daml.lf.language.{Ast, LookupError}
import com.daml.lf.transaction.{GlobalKey, NodeId, SubmittedTransaction}
import com.daml.lf.value.Value.{ContractId, ContractInst}
import com.daml.lf.speedy._
import com.daml.lf.speedy.SResult._
Expand Down Expand Up @@ -123,8 +123,8 @@ final case class ScenarioRunner(
}
}

case SResultNeedPackage(pkgId, _) =>
crash(s"package $pkgId not found")
case SResultNeedPackage(pkgId, context, _) =>
crash(LookupError.MissingPackage.pretty(pkgId, context))

case _: SResultNeedContract =>
crash("SResultNeedContract outside of submission")
Expand Down Expand Up @@ -443,8 +443,8 @@ object ScenarioRunner {
case SResultNeedTime(callback) =>
callback(ledger.currentTime)
go()
case SResultNeedPackage(pkgId, _) =>
throw Error.Internal(s"package $pkgId not found")
case SResultNeedPackage(pkgId, context, _) =>
throw Error.Internal(LookupError.MissingPackage.pretty(pkgId, context))
case _: SResultScenarioGetParty =>
throw Error.Internal("SResultScenarioGetParty in submission")
case _: SResultScenarioPassTime =>
Expand Down

0 comments on commit 73290c2

Please sign in to comment.