From 44e40d4f55f47cf5836f6b322c49911948f8da68 Mon Sep 17 00:00:00 2001 From: Gerald Monaco Date: Wed, 12 Feb 2020 20:28:31 -0800 Subject: [PATCH] Clean up async in next-server (#10476) * Clean up async in next-server * fix case when err is null * Refactor how components are loaded Co-authored-by: Joe Haddad --- .../next/next-server/server/next-server.ts | 249 ++++++++---------- 1 file changed, 115 insertions(+), 134 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 29e875557b80df6..58531b826d75997 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -70,6 +70,11 @@ type Middleware = ( next: (err?: Error) => void ) => void +type FindComponentsResult = { + components: LoadComponentsReturnType + query: ParsedUrlQuery +} + export type ServerConstructor = { /** * Where the Next project is located - @default '.' @@ -228,7 +233,7 @@ export default class Server { console.error(err) } - private handleRequest( + private async handleRequest( req: IncomingMessage, res: ServerResponse, parsedUrl?: UrlWithParsedQuery @@ -255,11 +260,13 @@ export default class Server { } res.statusCode = 200 - return this.run(req, res, parsedUrl).catch(err => { + try { + return await this.run(req, res, parsedUrl) + } catch (err) { this.logError(err) res.statusCode = 500 res.end('Internal Server Error') - }) + } } public getRequestHandler() { @@ -780,28 +787,36 @@ export default class Server { private async findPageComponents( pathname: string, - query: ParsedUrlQuery = {} - ) { - const serverless = !this.renderOpts.dev && this._isLikeServerless - // try serving a static AMP version first - if (query.amp) { + query: ParsedUrlQuery = {}, + params: Params | null = null + ): Promise { + const paths = [ + // try serving a static AMP version first + query.amp ? normalizePagePath(pathname) + '.amp' : null, + pathname, + ].filter(Boolean) + for (const pagePath of paths) { try { - return await loadComponents( + const components = await loadComponents( this.distDir, this.buildId, - normalizePagePath(pathname) + '.amp', - serverless + pagePath!, + !this.renderOpts.dev && this._isLikeServerless ) + return { + components, + query: { + ...(components.unstable_getStaticProps + ? { _nextDataReq: query._nextDataReq } + : query), + ...(params || {}), + }, + } } catch (err) { if (err.code !== 'ENOENT') throw err } } - return await loadComponents( - this.distDir, - this.buildId, - pathname, - serverless - ) + return null } private __sendPayload( @@ -847,8 +862,7 @@ export default class Server { req: IncomingMessage, res: ServerResponse, pathname: string, - query: ParsedUrlQuery = {}, - result: LoadComponentsReturnType, + { components, query }: FindComponentsResult, opts: any ): Promise { // we need to ensure the status code if /404 is visited directly @@ -857,16 +871,16 @@ export default class Server { } // handle static page - if (typeof result.Component === 'string') { - return result.Component + if (typeof components.Component === 'string') { + return components.Component } // check request state const isLikeServerless = - typeof result.Component === 'object' && - typeof (result.Component as any).renderReqToHTML === 'function' - const isSSG = !!result.unstable_getStaticProps - const isServerProps = !!result.unstable_getServerProps + typeof components.Component === 'object' && + typeof (components.Component as any).renderReqToHTML === 'function' + const isSSG = !!components.unstable_getStaticProps + const isServerProps = !!components.unstable_getServerProps // Toggle whether or not this is a Data request const isDataReq = query._nextDataReq @@ -888,7 +902,7 @@ export default class Server { // handle serverless if (isLikeServerless) { if (isDataReq) { - const renderResult = await (result.Component as any).renderReqToHTML( + const renderResult = await (components.Component as any).renderReqToHTML( req, res, true @@ -903,12 +917,12 @@ export default class Server { return null } this.prepareServerlessUrl(req, query) - return (result.Component as any).renderReqToHTML(req, res) + return (components.Component as any).renderReqToHTML(req, res) } if (isDataReq && isServerProps) { const props = await renderToHTML(req, res, pathname, query, { - ...result, + ...components, ...opts, isDataReq, }) @@ -917,7 +931,7 @@ export default class Server { } return renderToHTML(req, res, pathname, query, { - ...result, + ...components, ...opts, }) } @@ -968,7 +982,7 @@ export default class Server { let renderResult // handle serverless if (isLikeServerless) { - renderResult = await (result.Component as any).renderReqToHTML( + renderResult = await (components.Component as any).renderReqToHTML( req, res, true @@ -979,7 +993,7 @@ export default class Server { sprRevalidate = renderResult.renderOpts.revalidate } else { const renderOpts = { - ...result, + ...components, ...opts, } renderResult = await renderToHTML(req, res, pathname, query, renderOpts) @@ -1031,10 +1045,10 @@ export default class Server { query.__nextFallback = 'true' if (isLikeServerless) { this.prepareServerlessUrl(req, query) - html = await (result.Component as any).renderReqToHTML(req, res) + html = await (components.Component as any).renderReqToHTML(req, res) } else { html = (await renderToHTML(req, res, pathname, query, { - ...result, + ...components, ...opts, })) as string } @@ -1043,36 +1057,31 @@ export default class Server { this.__sendPayload(res, html, 'text/html; charset=utf-8') } - return doRender(ssgCacheKey, []).then( - async ({ isOrigin, value: { html, pageData, sprRevalidate } }) => { - // Respond to the request if a payload wasn't sent above (from cache) - if (!isResSent(res)) { - this.__sendPayload( - res, - isDataReq ? JSON.stringify(pageData) : html, - isDataReq ? 'application/json' : 'text/html; charset=utf-8', - sprRevalidate - ) - } - - // Update the SPR cache if the head request - if (isOrigin) { - // Preview mode should not be stored in cache - if (!isPreviewMode) { - await setSprCache( - ssgCacheKey, - { html: html!, pageData }, - sprRevalidate - ) - } - } + const { + isOrigin, + value: { html, pageData, sprRevalidate }, + } = await doRender(ssgCacheKey, []) + if (!isResSent(res)) { + this.__sendPayload( + res, + isDataReq ? JSON.stringify(pageData) : html, + isDataReq ? 'application/json' : 'text/html; charset=utf-8', + sprRevalidate + ) + } - return null + // Update the SPR cache if the head request + if (isOrigin) { + // Preview mode should not be stored in cache + if (!isPreviewMode) { + await setSprCache(ssgCacheKey, { html: html!, pageData }, sprRevalidate) } - ) + } + + return null } - public renderToHTML( + public async renderToHTML( req: IncomingMessage, res: ServerResponse, pathname: string, @@ -1085,69 +1094,54 @@ export default class Server { hasAmp?: boolean } = {} ): Promise { - return this.findPageComponents(pathname, query) - .then( - result => { - return this.renderToHTMLWithComponents( - req, - res, - pathname, - result.unstable_getStaticProps - ? { _nextDataReq: query._nextDataReq } - : query, - result, - { ...this.renderOpts, amphtml, hasAmp } - ) - }, - err => { - if (err.code !== 'ENOENT' || !this.dynamicRoutes) { - return Promise.reject(err) - } + try { + const result = await this.findPageComponents(pathname, query) + if (result) { + return await this.renderToHTMLWithComponents( + req, + res, + pathname, + result, + { ...this.renderOpts, amphtml, hasAmp } + ) + } - for (const dynamicRoute of this.dynamicRoutes) { - const params = dynamicRoute.match(pathname) - if (!params) { - continue - } + if (this.dynamicRoutes) { + for (const dynamicRoute of this.dynamicRoutes) { + const params = dynamicRoute.match(pathname) + if (!params) { + continue + } - return this.findPageComponents(dynamicRoute.page, query).then( - result => { - return this.renderToHTMLWithComponents( - req, - res, - dynamicRoute.page, - // only add params for SPR enabled pages - { - ...(result.unstable_getStaticProps - ? { _nextDataReq: query._nextDataReq } - : query), - ...params, - }, - result, - { - ...this.renderOpts, - params, - amphtml, - hasAmp, - } - ) + const result = await this.findPageComponents( + dynamicRoute.page, + query, + params + ) + if (result) { + return await this.renderToHTMLWithComponents( + req, + res, + dynamicRoute.page, + result, + { + ...this.renderOpts, + params, + amphtml, + hasAmp, } ) } - - return Promise.reject(err) } - ) - .catch(err => { - if (err && err.code === 'ENOENT') { - res.statusCode = 404 - return this.renderErrorToHTML(null, req, res, pathname, query) - } else { - this.logError(err) - res.statusCode = 500 - return this.renderErrorToHTML(err, req, res, pathname, query) - } - }) + } + } catch (err) { + this.logError(err) + res.statusCode = 500 + return await this.renderErrorToHTML(err, req, res, pathname, query) + } + + res.statusCode = 404 + return await this.renderErrorToHTML(null, req, res, pathname, query) } public async renderError( @@ -1175,7 +1169,7 @@ export default class Server { _pathname: string, query: ParsedUrlQuery = {} ) { - let result: null | LoadComponentsReturnType = null + let result: null | FindComponentsResult = null const { static404, pages404 } = this.nextConfig.experimental const is404 = res.statusCode === 404 @@ -1184,26 +1178,14 @@ export default class Server { // use static 404 page if available and is 404 response if (is404) { if (static404) { - try { - result = await this.findPageComponents('/_errors/404') - } catch (err) { - if (err.code !== 'ENOENT') { - throw err - } - } + result = await this.findPageComponents('/_errors/404') } // use 404 if /_errors/404 isn't available which occurs // during development and when _app has getInitialProps if (!result && pages404) { - try { - result = await this.findPageComponents('/404') - using404Page = true - } catch (err) { - if (err.code !== 'ENOENT') { - throw err - } - } + result = await this.findPageComponents('/404') + using404Page = result !== null } } @@ -1217,8 +1199,7 @@ export default class Server { req, res, using404Page ? '/404' : '/_error', - query, - result, + result!, { ...this.renderOpts, err,