-
Notifications
You must be signed in to change notification settings - Fork 10
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
Doesn't serve error stylesheet #15
Comments
I'll look at this as a way of getting into the code, assign it my way if you'd like. |
Awesome. I've given you push access as well :) |
I think I'll need to go through the code and comment a bit more. There are a few places where I've skipped explanations that might be needed to understand the problems that are solved |
That would be great - I'd been thinking about doing the same myself as I On Wed, 30 Sep 2015 12:11 Peter Müller notifications@github.com wrote:
|
Do you have objections to some quite a lot more extensive logging in verbose mode? |
Reproduced case, css output is: body {
background-color: red; }
/*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIi9Vc2Vycy9qYWNrL3RtcC9zcmMvdGVzdC5zY3NzIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUVBO0VBRlEsc0JBQUEsRUFBQSIsImZpbGUiOiJ0by5jc3MifQ== */er: 4px dashed red;
margin-bottom: 10px;
content: "Transpiler error: \00002fUsers\00002fjack\00002ftmp\00002fsrc\00002ftest\00002escss:4:30\00000aproperty \000022asasa\000022 must be followed by a \000027:\000027";
} Output from my new logging in 13ae910 is:
So what happens is that the cached content of N bytes gets returned, and then on the next read at offset N we get the error CSS and whack it into the buffer as well, which is what ends us up with the concatenated output above. |
In verbose mode we should have all the logging we need right now to get the show on the road. I'll refactor later to have it emit instead of logging directly, and then make up some more detailed verbosity levels to expose through the cli |
Nice reproduction. Seems like my structuring of the compiler error handling might be wrong. Might make sense to move the compiler fail handling to somewhere closer to the read function to be able to determine success or not. |
Coming back to this after a little time off - I'm still a little unsure of the structure here but I'm not sure this is to do with error handling - the error happens after the first read is entirely finished and the output written to buffer already. The problem is that that first read is given a cached version from the previous read when it shouldn't surely? |
Yeah, there's something with the caching handling the change from success to error wrong, making the length beign reported wrong. This is one of the tricky ones. I've had to deal a lot with the concept of reported length from the stat operation, which is what defines how much content to read. But the length is unknown at the time of statting, since the file hasn't been compiled yet. If we can just construct a test case that fails here then I'm sure we can start poking around to fix it more easily |
Expected: It should serve a CSS error stylesheet with the error message in content
Actual: Serves cached content, but the amount of bytes served is different than original
Seems like the cache should either be busted when the file is updated (see #21) or there is a missing compile promise rejection handler that should bust the cache before continuing.
The text was updated successfully, but these errors were encountered: