Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust the DB check error message #1558

Merged
merged 8 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions waspc/cli/src/Wasp/Cli/Command/Require.hs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import qualified System.FilePath as FP
import Wasp.Cli.Command (CommandError (CommandError), Requirable (checkRequirement), require)
import Wasp.Cli.Common (WaspProjectDir)
import qualified Wasp.Cli.Common as Cli.Common
import Wasp.Generator.DbGenerator.Operations (isDbRunning)
import Wasp.Generator.DbGenerator.Operations (isDbConnectionEstablished, testDbConnection)

data DbConnectionEstablished = DbConnectionEstablished deriving (Typeable)

Expand All @@ -49,7 +49,8 @@ instance Requirable DbConnectionEstablished where
-- call to 'require' will not result in an infinite loop.
InWaspProject waspProjectDir <- require
let outDir = waspProjectDir SP.</> Cli.Common.dotWaspDirInWaspProjectDir SP.</> Cli.Common.generatedCodeDirInDotWaspDir
dbIsRunning <- liftIO $ isDbRunning outDir
dbIsRunning <- liftIO $ isDbConnectionEstablished <$> testDbConnection outDir

if dbIsRunning
then return DbConnectionEstablished
else throwError noDbError
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ CREATE TABLE "User" (
"id" SERIAL NOT NULL,
"username" TEXT NOT NULL,
"password" TEXT NOT NULL,
"address" TEXT,

CONSTRAINT "User_pkey" PRIMARY KEY ("id")
);
Expand Down
7 changes: 7 additions & 0 deletions waspc/examples/crud-testing/src/client/vite.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from "vite";

export default defineConfig({
server: {
open: false
}
})
47 changes: 39 additions & 8 deletions waspc/src/Wasp/Generator/DbGenerator/Operations.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ module Wasp.Generator.DbGenerator.Operations
areAllMigrationsAppliedToDb,
dbReset,
dbSeed,
isDbRunning,
testDbConnection,
isDbConnectionEstablished,
containsDatabaseNotCreatedError,
)
where

Expand All @@ -17,10 +19,12 @@ import Control.Monad (when)
import Control.Monad.Catch (catch)
import Control.Monad.Extra (whenM)
import Data.Either (isRight)
import qualified Data.Text as T
import qualified Path as P
import StrongPath (Abs, Dir, File, Path', Rel, (</>))
import qualified StrongPath as SP
import System.Exit (ExitCode (..))
import qualified Text.Regex.TDFA as TR
import Wasp.Generator.Common (ProjectRootDir)
import Wasp.Generator.DbGenerator.Common
( DbSchemaChecksumFile,
Expand All @@ -38,13 +42,23 @@ import Wasp.Generator.DbGenerator.Common
import qualified Wasp.Generator.DbGenerator.Jobs as DbJobs
import Wasp.Generator.FileDraft.WriteableMonad (WriteableMonad (copyDirectoryRecursive))
import qualified Wasp.Generator.Job as J
import Wasp.Generator.Job.IO (printJobMsgsUntilExitReceived, readJobMessagesAndPrintThemPrefixed)
import Wasp.Generator.Job.IO
( collectJobTextOutputUntilExitReceived,
printJobMsgsUntilExitReceived,
readJobMessagesAndPrintThemPrefixed,
)
import qualified Wasp.Generator.WriteFileDrafts as Generator.WriteFileDrafts
import Wasp.Project.Db.Migrations (DbMigrationsDir)
import Wasp.Util (checksumFromFilePath, hexToString)
import Wasp.Util.IO (deleteFileIfExists, doesFileExist)
import qualified Wasp.Util.IO as IOUtil

data DbConnectionTestResult
= DbConnectionSuccess
| DbNotCreated
| DbConnectionFailure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be tempted to add String to DbConnectionFailure that contains actual error message, is there a reason why you decided to drop that information?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't get an error message per se, but we get some command output that might be useless unless parsed.

deriving (Eq)

-- | Migrates in the generated project context and then copies the migrations dir back
-- up to the wasp project dir to ensure they remain in sync.
migrateDevAndCopyToSource :: Path' Abs (Dir DbMigrationsDir) -> Path' Abs (Dir ProjectRootDir) -> MigrateArgs -> IO (Either String ())
Expand Down Expand Up @@ -135,15 +149,32 @@ dbSeed genProjectDir seedName = do
ExitSuccess -> Right ()
ExitFailure c -> Left $ "Failed with exit code " <> show c

isDbRunning ::
testDbConnection ::
Path' Abs (Dir ProjectRootDir) ->
IO Bool
isDbRunning genProjectDir = do
IO DbConnectionTestResult
testDbConnection genProjectDir = do
chan <- newChan
exitCode <- DbJobs.dbExecuteTest genProjectDir chan
-- NOTE: We only care if the command succeeds or fails, so we don't look at
-- the exit code or stdout/stderr for the process.
return $ exitCode == ExitSuccess

Martinsos marked this conversation as resolved.
Show resolved Hide resolved
case exitCode of
ExitSuccess -> return DbConnectionSuccess
ExitFailure _ -> do
outputLines <- collectJobTextOutputUntilExitReceived chan
let databaseNotCreated = any containsDatabaseNotCreatedError outputLines

return $
if databaseNotCreated
then DbNotCreated
else DbConnectionFailure

-- Prisma error code for "Database not created" is P1003.
containsDatabaseNotCreatedError :: T.Text -> Bool
containsDatabaseNotCreatedError text = text TR.=~ ("\\bP1003\\b" :: String)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably put this in where under the testDbConnection. But maybe you took it out so you can write tests for it? If so, I understand. But then I would give it a bit more descriptive name: e.g. prismaOutputContainsDbNotCreatedError, because now it is missing the context it would have if it was in the where.


isDbConnectionEstablished :: DbConnectionTestResult -> Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get what you do with this function, makes sense! I wonder a bit about name -> so we call that "connection established"? Even if database does not exist? It is not a bad name, but I wonder if it is precise enough. I guess the problem is I am not sure what "db connection established" really means.
Is there a better matching name? Maybe isDbInValidState? or isDbReady? isDbInUsableState? isDbReadyForPrisma? Something in that direction? If we can't find anything better, fine, let's leave it as it is, but I do feel that isDbConnectionEstablished could maybe be better.
If nothing, you could go with didDbConnectionTestSucceed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've went with isDbConnectionPossible since that's the check I was going for. The DB server can be connected to.

isDbConnectionEstablished DbConnectionSuccess = True
isDbConnectionEstablished DbNotCreated = True
isDbConnectionEstablished _ = False

generatePrismaClients :: Path' Abs (Dir ProjectRootDir) -> IO (Either String ())
generatePrismaClients projectRootDir = do
Expand Down
11 changes: 11 additions & 0 deletions waspc/src/Wasp/Generator/Job/IO.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ module Wasp.Generator.Job.IO
( readJobMessagesAndPrintThemPrefixed,
printJobMessage,
printJobMsgsUntilExitReceived,
collectJobTextOutputUntilExitReceived,
)
where

import Control.Concurrent (Chan, readChan)
import Control.Monad.IO.Class (liftIO)
import Data.Text (Text)
import qualified Data.Text.IO as T.IO
import System.IO (hFlush)
import qualified Wasp.Generator.Job as J
Expand All @@ -29,6 +31,15 @@ readJobMessagesAndPrintThemPrefixed chan = runPrefixedWriter go
J.JobOutput {} -> printJobMessagePrefixed jobMsg >> go
J.JobExit {} -> return ()

collectJobTextOutputUntilExitReceived :: Chan J.JobMessage -> IO [Text]
collectJobTextOutputUntilExitReceived = collect []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In haskell, usual idiom is to go with go, for functions like collect, as a name.

where
collect jobTextOutput chan = do
jobMsg <- readChan chan
case J._data jobMsg of
J.JobExit {} -> return jobTextOutput
J.JobOutput text _ -> collect (text : jobTextOutput) chan

printJobMessage :: J.JobMessage -> IO ()
printJobMessage jobMsg = do
let outHandle = getJobMessageOutHandle jobMsg
Expand Down
20 changes: 19 additions & 1 deletion waspc/test/Generator/DbGeneratorTest.hs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
module Generator.DbGeneratorTest where

import Test.Tasty.Hspec (Spec, it, shouldBe)
import qualified Data.Text as T
import Test.Tasty.Hspec (Spec, describe, it, shouldBe)
import Wasp.Generator.DbGenerator.Common
( MigrateArgs (..),
defaultMigrateArgs,
)
import Wasp.Generator.DbGenerator.Jobs (asPrismaCliArgs)
import Wasp.Generator.DbGenerator.Operations (containsDatabaseNotCreatedError)

spec_Jobs :: Spec
spec_Jobs =
Expand All @@ -19,3 +21,19 @@ spec_Jobs =
`shouldBe` ["--name", "something else longer"]
asPrismaCliArgs (MigrateArgs {_migrationName = Just "something", _isCreateOnlyMigration = True})
`shouldBe` ["--create-only", "--name", "something"]

spec_DbConnectionTest :: Spec
spec_DbConnectionTest =
describe "containsDatabaseNotCreatedError" $ do
it "should not match DB server not available error" $ do
containsDatabaseNotCreatedError
(T.pack "Error: P1001\n\nCan't reach database server at `localhost`:`5432`\n\nPlease make sure your database server is running at `localhost`:`5432`.")
`shouldBe` False
it "should not match similar error codes" $ do
containsDatabaseNotCreatedError
(T.pack "Error: P10033\n\nMade up error code")
`shouldBe` False
it "should match the DB not created error code" $ do
containsDatabaseNotCreatedError
(T.pack "Error: P1003\n\nDatabase `x` does not exist on the database server at `localhost:5432`.")
`shouldBe` True
Loading