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

add dynamic errorhandling #83

Closed
wants to merge 17 commits into from
Closed

Conversation

HHHartmann
Copy link
Contributor

@HHHartmann HHHartmann commented Apr 23, 2017

You can define static files as errorhandlers based on errorcode and file extension.

The last commits add support for six cars (or more) without crash. I serve static files without coroutine now. Very memory effichient!

Tested all sample pages successfully.

Also fix and improve many small details:

HHHartmann and others added 15 commits April 14, 2017 08:17
Add new todo which I am working on already. Basically to learn contributing
In AP mode wifi.sta.getip is nil
if you give the MIME type it will be mapped to text/plain
…HTTP Status string

Add all HTTP response codes used in the project
allow extra headers to be passed
return the response code string e.g.: return "Not found" for code 404
(needed in new error implementation)
now uses httpserver-error.lua to print the header
allow individual errorpages, selected based on http result code and
fille extension.
The parameter should be the connection itself. Log then calls getpeer on
it
sometimes I observe requests which are malformed (seem to be binary
data) so everything that does not have a method and request (url)
defined gets kicked.
make cars dynamic to load 1 to 4 cars
plus some more commented out prints
@ATAMAH
Copy link
Contributor

ATAMAH commented Apr 24, 2017

For starting: List of MIME types

You can't change "text/html" to just "html" and "application/json" to "json".

@HHHartmann
Copy link
Contributor Author

Thanks for reviewing the changes.
httpserver-header.lc has to be called with the file extension as 3rd parameter and then changes it to the corrosponding MIME type. So "json" becomes "application/json" and "html" becomes "text/html".
Calling it with "application/json" would result in the fallback MIME type "text/plain".

@@ -1,21 +0,0 @@
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
Copy link
Owner

Choose a reason for hiding this comment

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

I'd like to keep this around, it's useful to show the limitations of the server. I'm all for teaching clients how to work around said limitations, but it should be in a separate file.

Someone was supposed to do a pull request in which the workaround is based on having the served page include javascript to request the car images serially. I can see us having both examples included.

Can you do a separate PR in which you show your cars.lua workaround and leave this one in place? I'd rather do one pull request per logical change, this one bundles several.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,

glad you find some time to have a closer look.

The old cars.html example will work as well with the changes herein. But if it is to try the other approach I can put it back in.
I just created the cars.lua since I was too lazy to change the sources every time I want to test a different number of cars. It generates pretty much the same page as the cars.html.

As to the big PR:
I developed offline and then tried to separate the chenges inseparate checkins. They build up on each other. Cause of that it would be quite difficult to extract each feature/fix in a separate PR.
Id like to avoid that.

@@ -1,5 +1,5 @@
local function sendAttr(connection, attr, val)
connection:send("<li><b>".. attr .. ":</b> " .. val .. "<br></li>\n")
connection:send("<li><b>".. attr .. ":</b> " .. (val or "nil") .. "<br></li>\n")
Copy link
Owner

Choose a reason for hiding this comment

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

this improvement could be its own PR as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do a separate PR here.

@@ -0,0 +1,48 @@
-- httpserver-connection
Copy link
Owner

Choose a reason for hiding this comment

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

this comment seems outdated...
Why rename the file, though? Can you explain the reasoning behind this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ITs not a rename but an alternative implementation of the connection overlay.
I changed sending static files to not use a coroutine as that is quite memory consuming.
So to still be able to use the header.lua I needed a buffer to collect the separate sends in a string and then send them out.
I then only store the filename and the current position to send the chunks of the file.

@HHHartmann
Copy link
Contributor Author

So I will close this PR and create separate ones

@HHHartmann HHHartmann closed this May 29, 2017
@HHHartmann HHHartmann deleted the master branch July 2, 2017 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants