Blitz: Clean up build error for `blitz start --production`

[flybayer]

1. Change a typescript file to have a TS error, like missing variable or something
2. Run yarn blitz start -p
3. The build fails and prints the right error, but it also prints a bunch of garbage that needs to be cleaned up.

~/c/b/e/store> blitz start -p
Warning: You have enabled experimental feature(s).
Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

Creating an optimized production build

Failed to compile.

./pages/admin/index.tsx
SyntaxError: /Users/b/c/blitz/examples/store/.blitz/caches/build/pages/admin/index.tsx: Unexpected token, expected "jsxTagEnd" (7:6)

   5 |     <div>
   6 |       <h1>Store Admin</h1
>  7 |       <div>
     |       ^
   8 |         <p>
   9 |           <Link href="/admin/products">
  10 |             <a>Manage Products</a>


> Build error occurred
Error: > Build failed because of webpack errors
    at build (/Users/b/c/blitz/node_modules/next/dist/build/index.js:13:900)
Warning: You have enabled experimental feature(s).
Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

Error: Could not find a valid build in the '/Users/b/c/blitz/examples/store/.next' directory! Try building your app with 'next build' before starting the server.
    at Server.readBuildId (/Users/b/c/blitz/node_modules/next/dist/next-server/server/next-server.js:941:23)
    at new Server (/Users/b/c/blitz/node_modules/next/dist/next-server/server/next-server.js:57:29)
    at createServer (/Users/b/c/blitz/node_modules/next/dist/server/next.js:2:133)
    at start (/Users/b/c/blitz/node_modules/next/dist/server/lib/start-server.js:1:323)
    at nextStart (/Users/b/c/blitz/node_modules/next/dist/cli/next-start.js:20:125)
    at /Users/b/c/blitz/node_modules/next/dist/bin/next:28:339

Here's everything that needs removed from the end:

Warning: You have enabled experimental feature(s).
Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

Error: Could not find a valid build in the '/Users/b/c/blitz/examples/store/.next' directory! Try building your app with 'next build' before starting the server.
    at Server.readBuildId (/Users/b/c/blitz/node_modules/next/dist/next-server/server/next-server.js:941:23)
    at new Server (/Users/b/c/blitz/node_modules/next/dist/next-server/server/next-server.js:57:29)
    at createServer (/Users/b/c/blitz/node_modules/next/dist/server/next.js:2:133)
    at start (/Users/b/c/blitz/node_modules/next/dist/server/lib/start-server.js:1:323)
    at nextStart (/Users/b/c/blitz/node_modules/next/dist/cli/next-start.js:20:125)
    at /Users/b/c/blitz/node_modules/next/dist/bin/next:28:339

The correct output should be:

~/c/b/e/store> blitz start -p
Warning: You have enabled experimental feature(s).
Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

Creating an optimized production build

Failed to compile.

./pages/admin/index.tsx
SyntaxError: /Users/b/c/blitz/examples/store/.blitz/caches/build/pages/admin/index.tsx: Unexpected token, expected "jsxTagEnd" (7:6)

   5 |     <div>
   6 |       <h1>Store Admin</h1
>  7 |       <div>
     |       ^
   8 |         <p>
   9 |           <Link href="/admin/products">
  10 |             <a>Manage Products</a>


> Build error occurred
Error: > Build failed because of webpack errors
    at build (/Users/b/c/blitz/node_modules/next/dist/build/index.js:13:900)

[merelinguist]

@camilo86 I haven’t fixed this yet but if you’re working on it the problem is in prod.ts:

// packages/server/src/prod.ts

import {ServerConfig, enhance} from './config'
import {nextStart} from './next-utils'
import {build} from './build'

export async function prod(config: ServerConfig) {
  const {rootFolder, nextBin} = await enhance(config)
  await build(config)
  await nextStart(nextBin, rootFolder)
}

await build(config) gives the first (wanted) part of the error, and await nextStart(nextBin, rootFolder) gives the unwanted part.

If the file looked like this:

// packages/server/src/prod.ts

import {ServerConfig} from './config'
import {build} from './build'

export async function prod(config: ServerConfig) {
  await build(config)
}

It would gives us the ouput we want.

To fix, you’ll have to find some way to only run nextStart if build succeeds. Good luck! 👍

[ryardley]

@camilo86 We don't really have access to next's errors as it is running in a child process and need to alter it's output in this particular case.

So far what we have done is alter error output messages but this is a hack and is fairly brittle. We will want to be careful with this.

https://github.com/blitz-js/blitz/blob/canary/packages/server/src/next-utils.ts#L6
https://github.com/blitz-js/blitz/blob/canary/packages/server/src/next-utils.ts#L34

We will probably want to have a layer that just handles next-js output. I am not sure of a better way to get into next's internal state although if we can import a programmatic interface instead of running a bin we might have more control.

In this instance I would probably create a module and pipe the output from next to that module and add transforms to change the output there.

import outputStream from './next-output';

//...

pty.spawn(nextBin, ['dev'], { cwd }).pipe(outputStream)

As far as other errors within code we control (not really this issue) I think we would probably want to have our own special error handler that reformats errors when they arise. I wonder if at a later point some of the invariant work you were doing @merelinguist could be refactored so that it caught specific errors and displayed them nicely? That could be a better way to design this sort of code.

try {
  await build(config);
} catch(err) {
  displayError(err); // Analyses the type of Error and displays the appropriate message.
}

[merelinguist]

@ryardley Blitz relies on a lot of external packages that use clis (prisma, next) which will probably lead to a bit of an inconsistent experience. I think as a rule we should aim to do as much of the actual logging of information / errors ourselves. I’m going to write up a pr (or at least a design issue) soon implementing some kind of standardised way for us to print stuff.

[ryardley]

After some more thought I updated my answer above as whoever takes this on needs to have an idea of what we have done and why. @merelinguist controlling subprocess output may be difficult depending on the sub process as we don't have insight into the child processes we run unless it is something designed to be serializable like webpack. Ideally we would import a programmatic node interface and run it in our own childprocess and handle the errors ourselves. Perhaps next exposes that capability somewhere?

[camilo86]

@ryardley So far, from investigating the issue; seems like build(config) fails without throwing any errors causing nextStart(nextBin, rootFolder) to get executed.

I did get build(config) to throw errors when the process fails by checking the status code of the spawn process:

export async function nextBuild(nextBin: string, cwd: string) {
  return new Promise((res, rej) => {
    spawn(nextBin, ['build'], {
      cwd,
      stdio: 'inherit',
    })
      .on('exit',  (code) => {
        code === 0 ? res() : rej(`Failed with status code: ${code}`)
      })
      .on('error', rej)
  })
}

Now, the solution above only lets us know if the process finished successfully/failed and does not provide any specific error message from the process. What do you all think?

[flybayer]

Hmmm, how long until we fork Next.js? 🤔🙂

I totally agree about wanting to have more control over the error display for a more consistent experience. I'm excited to see y'all working on this.

I think we probably can start Next programmatically without forking. Basically implement the following in our own code:

• packages/next/cli/next-dev.ts
• packages/next/cli/next-start.ts

[flybayer]

We could also do the same for Prisma — import and use node functions instead of spawning the CLI. If it makes sense to.

https://github.com/prisma/prisma/blob/master/cli/prisma2/src/bin.ts

[flybayer]

A related problem we have right now is that blitz commands are returning status 0 even when spawned processes return 1