Skip to content

Commit

Permalink
wrong redirect in local server (mdn#2179)
Browse files Browse the repository at this point in the history
* wrong redirect in local server

Part of #350

* end-to-end tests
  • Loading branch information
peterbe authored and SphinxKnight committed Dec 20, 2020
1 parent 74fc45f commit 32de49d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 15 deletions.
19 changes: 4 additions & 15 deletions server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,34 +192,23 @@ app.get("/*", async (req, res) => {
clearKumascriptRenderCache: true,
});
console.timeEnd(`buildDocumentFromURL(${lookupURL})`);
if (!built) {
return res
.status(404)
.sendFile(path.join(STATIC_ROOT, "en-us", "_spas", "404.html"));
if (built) {
document = built.doc;
bcdData = built.bcdData;
}
document = built.doc;
bcdData = built.bcdData;
} catch (error) {
console.error(`Error in buildDocumentFromURL(${lookupURL})`, error);
return res.status(500).send(error.toString());
}

if (!document) {
// redirect resolving can take some time, so we only do it when there's no document
// for the current route
const redirectURL = Redirect.resolve(lookupURL);
if (redirectURL !== lookupURL) {
return res.redirect(301, redirectURL + extraSuffix);
}

// It doesn't resolve to a file on disk and it's not a redirect.
// Try to send a slightly better error at least.
return res
.status(404)
.send(
`From URL ${lookupURL} no folder on disk could be found. ` +
`Tried to find a folder called ${Document.urlToFolderPath(lookupURL)}`
);
.sendFile(path.join(STATIC_ROOT, "en-us", "_spas", "404.html"));
}

if (bcdDataURL) {
Expand Down
30 changes: 30 additions & 0 deletions testing/tests/developing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,36 @@ describe("Testing the Express server", () => {
expect(response.headers.location).toBe("/en-US/docs/Web");
});

withDeveloping("redirect based on _redirects.txt", async () => {
// Yes, this is a bit delicate since it depends on non-fixtures, but
// it's realistic and it's a good end-to-end test.
// See mdn/content/files/en-us/_redirects.txt

// First redirect *out* to an external URL.
let response = await got(
serverURL(
"/en-US/docs/Mozilla/Add-ons/WebExtensions/Publishing_your_WebExtension"
),
{ followRedirect: false }
);
expect(response.statusCode).toBe(301);
expect(response.headers.location).toBe(
"https://extensionworkshop.com/documentation/publish/package-your-extension/"
);

// Redirect within.
response = await got(
serverURL(
"/en-US/docs/Mozilla/Add-ons/WebExtensions/Extension_API_differences"
),
{ followRedirect: false }
);
expect(response.statusCode).toBe(301);
expect(response.headers.location).toBe(
"/en-US/docs/Mozilla/Add-ons/WebExtensions/Differences_between_API_implementations"
);
});

withDeveloping("redirect by preferred locale cookie", async () => {
let response = await got(serverURL("/"), {
followRedirect: false,
Expand Down

0 comments on commit 32de49d

Please sign in to comment.