improve handling of malformed profile JSON files

In case any of the sc4pac-profiles.json, sc4pac-plugins.json or
sc4pac-plugins-lock.json files are malformed (JSON fails to parse), this
error is now handled appropriately without crashing the sc4pac process.

Resolves memo33/sc4pac-gui#4

CHANGELOG.md

@@ -5,7 +5,8 @@
 - Added `--launch-browser` option to the `server` command, which opens the web-app in the browser on start-up ([#3][gui3]).
 
 ### Fixed
-- an issue where building a channel with file names containing spaces failed
+- an issue where building a channel with file names containing spaces failed.
+- Handling of malformed profile JSON files has been improved ([#4][gui4]).
 - The auto-shutdown functionality of the server now handles multiple connections and page reloads of the web-app ([#10][gui10]).
 
 ### Changed
@@ -17,6 +18,7 @@
 
 
 [gui3]: https://github.com/memo33/sc4pac-gui/issues/3
+[gui4]: https://github.com/memo33/sc4pac-gui/issues/4
 [gui10]: https://github.com/memo33/sc4pac-gui/issues/10
 [gui14]: https://github.com/memo33/sc4pac-gui/issues/14
 

api.md

@@ -47,7 +47,8 @@ GET  /image.fetch?url=<url>
   - 400 (incorrect input)
   - 404 (non-existing packages, assets, etc.)
   - 409 `/error/profile-not-initialized` (when not initialized)
-  - 500 (unexpected unresolvable situations)
+  - 500 `/error/profile-read-error` (when one of the profile JSON files fails to parse)
+  - 500 (other unexpected unresolvable situations)
   - 502 (download failures)
 - Errors are of the form
   ```
@@ -70,6 +71,7 @@ Returns:
   The response contains
   `platformDefaults: {plugins: ["<path>", …], cache: ["<path>", …], temp: ["<path>", …]}`
   for recommended platform-specific locations to use for initialization.
+- 500 `/error/profile-read-error` when the profile JSON file exists, but does not have the expected format.
 
 ## profile.init
 
@@ -487,12 +489,14 @@ Get the list of all existing profiles, each corresponding to a Plugins folder.
 Synopsis: `GET /profiles.list`
 
 Returns:
-```
-{
-  profiles: [{id: "<id-1>", name: string}, …],
-  currentProfileId: ["<id-1>"]
-}
-```
+- 500 `/error/profile-read-error`
+- 200:
+  ```
+  {
+    profiles: [{id: "<id-1>", name: string}, …],
+    currentProfileId: ["<id-1>"]
+  }
+  ```
 
 ## profiles.add
 

src/main/scala/sc4pac/Data.scala

@@ -120,13 +120,17 @@ object JsonData extends SharedData {
       } yield data
     }
 
-    val read: ZIO[ProfileRoot, ErrStr, Plugins] = Plugins.pathURIO.flatMap { pluginsPath =>
-      val task: IO[ErrStr | java.io.IOException, Plugins] =
+    /** Reads the Plugins JSON file if it exists, otherwise returns None */
+    val readMaybe: ZIO[ProfileRoot, error.ReadingProfileFailed, Option[Plugins]] = Plugins.pathURIO.flatMap { pluginsPath =>
+      val task: IO[ErrStr | java.io.IOException, Option[Plugins]] =
         ZIO.ifZIO(ZIO.attemptBlockingIO(os.exists(pluginsPath)))(
-          onFalse = ZIO.fail(s"Configuration file does not exist: $pluginsPath"),
-          onTrue = ZIO.attemptBlockingIO(JsonIo.readBlocking[Plugins](pluginsPath)).absolve
+          onFalse = ZIO.succeed(None),
+          onTrue = ZIO.attemptBlockingIO(JsonIo.readBlocking[Plugins](pluginsPath)).absolve.map(Some(_))
         )
-      task.mapError(_.toString)
+      task.mapError(e => error.ReadingProfileFailed(
+        s"Failed to read profile JSON file ${pluginsPath.last}.",
+        f"Make sure the file is correctly formatted: $pluginsPath.%n$e"),
+      )
     }
 
     /** Read Plugins from file if it exists, else create it and write it to file. */
@@ -138,6 +142,10 @@ object JsonData extends SharedData {
           data                     <- Plugins.init(pluginsRoot, cacheRoot, tempRoot = defaultTempRoot)
         } yield data
       )
+      .mapError(e => error.ReadingProfileFailed(
+        s"Failed to read profile JSON file ${pluginsPath.last}.",
+        f"Make sure the file is correctly formatted: $pluginsPath.%n${e.getMessage}"),
+      )
     }
   }
 
@@ -251,6 +259,10 @@ object JsonData extends SharedData {
           JsonIo.write(pluginsLockPath, data, None)(ZIO.succeed(data))
         }
       )
+      .mapError(e => error.ReadingProfileFailed(
+        s"Failed to read profile lock file ${pluginsLockPath.last}.",
+        f"Make sure the file is correctly formatted: $pluginsLockPath.%n${e.getMessage}"),
+      )
     }
 
     // does *not* automatically upgrade from scheme 1 (this function is only used for reading, not writing)
@@ -382,6 +394,10 @@ object JsonData extends SharedData {
         onTrue = JsonIo.read[Profiles](jsonPath),
         onFalse = ZIO.succeed(Profiles(Seq.empty, None))
       )
+      .mapError(e => error.ReadingProfileFailed(
+        s"Failed to read profiles file ${jsonPath.last}.",
+        f"Make sure the file is correctly formatted: $jsonPath.%n${e.getMessage}"),
+      )
     }
   }
 

src/main/scala/sc4pac/api/api.scala

@@ -28,13 +28,16 @@ class Api(options: sc4pac.cli.Commands.ServerOptions) {
       "temp" -> Seq(JD.Plugins.defaultTempRoot.toString),
     )
 
-  /** Sends a 409 ProfileNotInitialized if Plugins cannot be loaded. */
+  /** Sends a 409 ProfileNotInitialized or 500 ReadingProfileFailed if Plugins cannot be loaded. */
   private val readPluginsOr409: ZIO[ProfileRoot, Response, JD.Plugins] =
-    JD.Plugins.read.flatMapError { (err: ErrStr) =>
-      for {
-        defaults <- makePlatformDefaults
-      } yield jsonResponse(ErrorMessage.ProfileNotInitialized("Profile not initialized", err, platformDefaults = defaults)).status(Status.Conflict)
-    }
+    JD.Plugins.readMaybe
+      .mapError((e: error.ReadingProfileFailed) => jsonResponse(expectedFailureMessage(e)).status(expectedFailureStatus(e)))
+      .someOrElseZIO((  // Plugins file does not exist
+        for {
+          defaults <- makePlatformDefaults
+          msg      =  ErrorMessage.ProfileNotInitialized("Profile not initialized.", "Profile JSON file does not exist.", platformDefaults = defaults)
+        } yield jsonResponse(msg).status(Status.Conflict)
+      ).flip)
 
   private def expectedFailureMessage(err: cli.Commands.ExpectedFailure): ErrorMessage = err match {
     case abort: error.Sc4pacVersionNotFound => ErrorMessage.VersionNotFound(abort.title, abort.detail)
@@ -44,6 +47,7 @@ class Api(options: sc4pac.cli.Commands.ServerOptions) {
     case abort: error.DownloadFailed => ErrorMessage.DownloadFailed(abort.title, abort.detail)
     case abort: error.ChecksumError => ErrorMessage.DownloadFailed(abort.title, abort.detail)
     case abort: error.ChannelsNotAvailable => ErrorMessage.ChannelsNotAvailable(abort.title, abort.detail)
+    case abort: error.ReadingProfileFailed => ErrorMessage.ReadingProfileFailed(abort.title, abort.detail)
     case abort: error.Sc4pacAbort => ErrorMessage.Aborted("Operation aborted.", "")
   }
 
@@ -55,6 +59,7 @@ class Api(options: sc4pac.cli.Commands.ServerOptions) {
     case abort: error.DownloadFailed => Status.BadGateway
     case abort: error.ChecksumError => Status.BadGateway
     case abort: error.ChannelsNotAvailable => Status.BadGateway
+    case abort: error.ReadingProfileFailed => Status.InternalServerError
     case abort: error.Sc4pacAbort => Status.Ok  // this is not really an error, but the expected control flow
   }
 
@@ -145,7 +150,7 @@ class Api(options: sc4pac.cli.Commands.ServerOptions) {
   /** Routes that require a `profile=id` query parameter as part of the URL. */
   def profileRoutes: Routes[ProfileRoot, Throwable] = Routes(
 
-    // 200, 409
+    // 200, 409, 500
     Method.GET / "profile.read" -> handler { (req: Request) =>
       wrapHttpEndpoint {
         for {
@@ -529,7 +534,7 @@ class Api(options: sc4pac.cli.Commands.ServerOptions) {
         } yield jsonOk
       }),
 
-      // 200
+      // 200, 500
       Method.GET / "profiles.list" -> handler {
         wrapHttpEndpoint {
           JD.Profiles.readOrInit.map(profiles => jsonResponse(profiles.copy(settings = ujson.Obj())))

src/main/scala/sc4pac/api/message.scala

@@ -92,6 +92,8 @@ object ErrorMessage {
   case class ServerError(title: String, detail: String) extends ErrorMessage derives UP.ReadWriter
   @upickle.implicits.key("/error/profile-not-initialized")
   case class ProfileNotInitialized(title: String, detail: String, platformDefaults: Map[String, Seq[String]]) extends ErrorMessage derives UP.ReadWriter
+  @upickle.implicits.key("/error/profile-read-error")
+  case class ReadingProfileFailed(title: String, detail: String) extends ErrorMessage derives UP.ReadWriter
   @upickle.implicits.key("/error/version-not-found")
   case class VersionNotFound(title: String, detail: String) extends ErrorMessage derives UP.ReadWriter
   @upickle.implicits.key("/error/asset-not-found")

src/main/scala/sc4pac/cli.scala

@@ -39,7 +39,7 @@ object Commands {
   // failures that are expected with both the CLI and the API
   type ExpectedFailure = error.Sc4pacAbort | error.DownloadFailed | error.ChannelsNotAvailable
     | error.Sc4pacVersionNotFound | error.Sc4pacAssetNotFound | error.ExtractionFailed
-    | error.UnsatisfiableVariantConstraints | error.ChecksumError
+    | error.UnsatisfiableVariantConstraints | error.ChecksumError | error.ReadingProfileFailed
 
   private def handleExpectedFailures(abort: ExpectedFailure, exit: Int => Nothing): Nothing = abort match {
     case abort: error.Sc4pacAbort => { System.err.println("Operation aborted."); exit(1) }

src/main/scala/sc4pac/error.scala

@@ -32,6 +32,8 @@ final class NotADbpfFile(val title: String, val detail: String) extends java.io.
 
 final class ChannelsNotAvailable(val title: String, val detail: String) extends java.io.IOException(s"$title $detail") with Sc4pacErr
 
+final class ReadingProfileFailed(val title: String, val detail: String) extends java.io.IOException(s"$title $detail") with Sc4pacErr
+
 final class SymlinkCreationFailed(msg: String) extends java.io.IOException(msg) with Sc4pacErr
 
 final class FileOpsFailure(msg: String) extends java.io.IOException(msg) with Sc4pacErr