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

Render 400-500 status response HTML #39

Merged
merged 1 commit into from
Dec 30, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion src/core/drive/form_submission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ export class FormSubmission {
}

requestSucceededWithResponse(request: FetchRequest, response: FetchResponse) {
if (this.requestMustRedirect(request) && !response.redirected) {
if (response.clientError || response.serverError) {
this.delegate.formSubmissionFailedWithResponse(this, response)
} else if (this.requestMustRedirect(request) && !response.redirected) {
const error = new Error("Form responses must redirect to another location")
Copy link

@santib santib Dec 28, 2020

Choose a reason for hiding this comment

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

What is the technical reason why we need to have this error triggered when the response is not a redirect?

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the technical reason why we need to have this error triggered when the response is not a redirect?

#22 (comment)

this.delegate.formSubmissionErrored(this, error)
} else {
Expand Down
12 changes: 10 additions & 2 deletions src/core/drive/navigator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { FetchResponse } from "../../http/fetch_response"
import { FormSubmission } from "./form_submission"
import { Locatable, Location } from "../location"
import { Visit, VisitDelegate, VisitOptions } from "./visit"
import { Snapshot } from "./snapshot"

export type NavigatorDelegate = VisitDelegate & {
allowsVisitingLocation(location: Location): boolean
Expand Down Expand Up @@ -87,8 +88,15 @@ export class Navigator {
}
}

formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {
console.error("Form submission failed", formSubmission, fetchResponse)
async formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {
const responseHTML = await fetchResponse.responseHTML

if (responseHTML) {
debugger
Copy link
Contributor

Choose a reason for hiding this comment

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

👻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤠

Copy link
Contributor Author

Choose a reason for hiding this comment

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

const snapshot = Snapshot.fromHTMLString(responseHTML)
this.view.render({ snapshot }, () => {})
this.view.clearSnapshotCache()
}
}

formSubmissionErrored(formSubmission: FormSubmission, error: Error) {
Expand Down
2 changes: 1 addition & 1 deletion src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class FrameController implements FetchRequestDelegate, FormInterceptorDel
}

formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {

this.element.controller.loadResponse(fetchResponse)
}

formSubmissionErrored(formSubmission: FormSubmission, error: Error) {
Expand Down
8 changes: 8 additions & 0 deletions src/http/fetch_response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ export class FetchResponse {
return !this.succeeded
}

get clientError() {
return this.statusCode >= 400 && this.statusCode <= 499
}

get serverError() {
return this.statusCode >= 500 && this.statusCode <= 599
}

get redirected() {
return this.response.redirected
}
Expand Down
13 changes: 13 additions & 0 deletions src/tests/fixtures/422.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<html>
<head>
<title>Unprocessable Entity</title>
<script src="/src/tests/fixtures/turbo.es2017-umd.js" data-turbo-track="reload"></script>
</head>
<body>
<h1>Unprocessable Entity</h1>

<turbo-frame id="frame">
<h2>Frame: Unprocessable Entity</h2>
</turbo-frame>
</body>
</html>
13 changes: 13 additions & 0 deletions src/tests/fixtures/500.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<html>
<head>
<title>Internal Server Error</title>
<script src="/src/tests/fixtures/turbo.es2017-umd.js" data-turbo-track="reload"></script>
</head>
<body>
<h1>Internal Server Error</h1>

<turbo-frame id="frame">
<h2>Frame: Internal Server Error</h2>
</turbo-frame>
</body>
</html>
18 changes: 18 additions & 0 deletions src/tests/fixtures/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@
<input type="submit">
</form>
</div>
<div id="reject">
<form class="unprocessable_entity" action="/__turbo/reject" method="post">
<input type="hidden" name="status" value="422">
<input type="submit">
</form>
<form class="internal_server_error" action="/__turbo/reject" method="post">
<input type="hidden" name="status" value="500">
<input type="submit">
</form>
</div>
<div id="submitter">
<form action="/src/tests/fixtures/one.html" method="get">
<button type="submit" formmethod="post" formaction="/__turbo/redirect"
Expand All @@ -24,6 +34,14 @@
<input type="hidden" name="path" value="/src/tests/fixtures/frames/form.html">
<input type="submit">
</form>
<form class="unprocessable_entity" action="/__turbo/reject" method="post">
<input type="hidden" name="status" value="422">
<input type="submit">
</form>
<form class="internal_server_error" action="/__turbo/reject" method="post">
<input type="hidden" name="status" value="500">
<input type="submit">
</form>
<form action="/__turbo/messages" method="post" class="stream">
<input type="hidden" name="type" value="stream">
<input type="hidden" name="content" value="Hello!">
Expand Down
36 changes: 36 additions & 0 deletions src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@ export class FormSubmissionTests extends TurboDriveTestCase {
this.assert.equal(await this.visitAction, "advance")
}

async "test invalid form submission with unprocessable entity status"() {
await this.clickSelector("#reject form.unprocessable_entity input[type=submit]")
await this.nextBody

const title = await this.querySelector("h1")
this.assert.equal(await title.getVisibleText(), "Unprocessable Entity", "renders the response HTML")
this.assert.notOk(await this.hasSelector("#frame form.reject"), "replaces entire page")
}

async "test invalid form submission with server error status"() {
await this.clickSelector("#reject form.internal_server_error input[type=submit]")
await this.nextBody

const title = await this.querySelector("h1")
this.assert.equal(await title.getVisibleText(), "Internal Server Error", "renders the response HTML")
this.assert.notOk(await this.hasSelector("#frame form.reject"), "replaces entire page")
}

async "test submitter form submission reads button attributes"() {
const button = await this.querySelector("#submitter form button[type=submit]")
await button.click()
Expand All @@ -34,6 +52,24 @@ export class FormSubmissionTests extends TurboDriveTestCase {
this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
}

async "test invalid frame form submission with unprocessable entity status"() {
await this.clickSelector("#frame form.unprocessable_entity input[type=submit]")
await this.nextBeat

const title = await this.querySelector("#frame h2")
this.assert.ok(await this.hasSelector("#reject form"), "only replaces frame")
this.assert.equal(await title.getVisibleText(), "Frame: Unprocessable Entity")
}

async "test invalid frame form submission with internal server errror status"() {
await this.clickSelector("#frame form.internal_server_error input[type=submit]")
await this.nextBeat

const title = await this.querySelector("#frame h2")
this.assert.ok(await this.hasSelector("#reject form"), "only replaces frame")
this.assert.equal(await title.getVisibleText(), "Frame: Internal Server Error")
}

async "test frame form submission with stream response"() {
const button = await this.querySelector("#frame form.stream input[type=submit]")
await button.click()
Expand Down
8 changes: 8 additions & 0 deletions src/tests/server.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Response, Router } from "express"
import multer from "multer"
import path from "path"

const router = Router()
const streamResponses: Set<Response> = new Set
Expand All @@ -11,6 +12,13 @@ router.post("/redirect", (request, response) => {
response.redirect(303, path)
})

router.post("/reject", (request, response) => {
const { status } = request.body
const fixture = path.join(__dirname, `../../src/tests/fixtures/${status}.html`)

response.status(parseInt(status || "422", 10)).sendFile(fixture)
})

router.post("/messages", (request, response) => {
const { content, type } = request.body
if (typeof content == "string") {
Expand Down