Skip to content

Commit

Permalink
Bug/issue 1102 not found static assets returning 500 for serve command (
Browse files Browse the repository at this point in the history
#1103)

* check static resource exists before serving it

* add test cases to cover expected 404 status code handling
  • Loading branch information
thescientist13 committed May 6, 2023
1 parent 06a9f3b commit 2ae137b
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 26 deletions.
55 changes: 29 additions & 26 deletions packages/cli/src/lifecycles/serve.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fs from 'fs/promises';
import { hashString } from '../lib/hashing-utils.js';
import Koa from 'koa';
import { mergeResponse } from '../lib/resource-utils.js';
import { checkResourceExists, mergeResponse } from '../lib/resource-utils.js';
import { Readable } from 'stream';
import { ResourceInterface } from '../lib/resource-interface.js';

Expand Down Expand Up @@ -227,34 +227,37 @@ async function getStaticServer(compilation, composable) {
app.use(async (ctx, next) => {
try {
const url = new URL(`.${ctx.url}`, outputDir.href);
const resourcePlugins = standardResourcePlugins
.filter((plugin) => plugin.isStandardStaticResource)
.map((plugin) => {
return plugin.provider(compilation);
});

const request = new Request(url.href, {
headers: new Headers(ctx.request.header)
});
const initResponse = new Response(ctx.body, {
status: ctx.response.status,
headers: new Headers(ctx.response.header)
});
const response = await resourcePlugins.reduce(async (responsePromise, plugin) => {
return plugin.shouldServe && await plugin.shouldServe(url, request)
? Promise.resolve(await plugin.serve(url, request))
: responsePromise;
}, Promise.resolve(initResponse));
if (await checkResourceExists(url)) {
const resourcePlugins = standardResourcePlugins
.filter((plugin) => plugin.isStandardStaticResource)
.map((plugin) => {
return plugin.provider(compilation);
});

if (response.ok) {
ctx.body = Readable.from(response.body);
ctx.type = response.headers.get('Content-Type');
ctx.status = response.status;
const request = new Request(url.href, {
headers: new Headers(ctx.request.header)
});
const initResponse = new Response(ctx.body, {
status: ctx.response.status,
headers: new Headers(ctx.response.header)
});
const response = await resourcePlugins.reduce(async (responsePromise, plugin) => {
return plugin.shouldServe && await plugin.shouldServe(url, request)
? Promise.resolve(await plugin.serve(url, request))
: responsePromise;
}, Promise.resolve(initResponse));

// TODO automatically loop and apply all custom headers to Koa response, include Content-Type below
// https://github.com/ProjectEvergreen/greenwood/issues/1048
if (response.headers.has('Content-Length')) {
ctx.set('Content-Length', response.headers.get('Content-Length'));
if (response.ok) {
ctx.body = Readable.from(response.body);
ctx.type = response.headers.get('Content-Type');
ctx.status = response.status;

// TODO automatically loop and apply all custom headers to Koa response, include Content-Type below
// https://github.com/ProjectEvergreen/greenwood/issues/1048
if (response.headers.has('Content-Length')) {
ctx.set('Content-Length', response.headers.get('Content-Length'));
}
}
}
} catch (e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,30 @@ describe('Serve Greenwood With: ', function() {
done();
});
});

describe('Serve command with API 404 not found behavior', function() {
let response = {};

before(async function() {
return new Promise((resolve, reject) => {
request.get(`${hostname}/api/foo`, (err, res, body) => {
if (err) {
reject();
}

response = res;
response.body = body;

resolve();
});
});
});

it('should return a 404 status', function(done) {
expect(response.statusCode).to.equal(404);
done();
});
});
});

after(function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,30 @@ describe('Serve Greenwood With: ', function() {
expect(scriptFiles.length).to.equal(2);
});
});

describe('Serve command with 404 not found behavior', function() {
let response = {};

before(async function() {
return new Promise((resolve, reject) => {
request.get(`${hostname}/foo`, (err, res, body) => {
if (err) {
reject();
}

response = res;
response.body = body;

resolve();
});
});
});

it('should return a 404 status', function(done) {
expect(response.statusCode).to.equal(404);
done();
});
});
});

after(function() {
Expand Down
24 changes: 24 additions & 0 deletions packages/cli/test/cases/serve.default/serve.default.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,30 @@ describe('Serve Greenwood With: ', function() {
});
});

describe('Serve command with 404 not found behavior', function() {
let response = {};

before(async function() {
return new Promise((resolve, reject) => {
request.get(`${hostname}/foo.png`, (err, res, body) => {
if (err) {
reject();
}

response = res;
response.body = body;

resolve();
});
});
});

it('should return a 404 status', function(done) {
expect(response.statusCode).to.equal(404);
done();
});
});

});

after(function() {
Expand Down

0 comments on commit 2ae137b

Please sign in to comment.