-
Notifications
You must be signed in to change notification settings - Fork 130
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
Initial test – three questions #2
Comments
Update: The first issue is probably not related to SEB 3.0. I installed SEB 2.4 again and the issue remained, and logging out and back in resolved it, even for 3.0. |
Thanks for testing and your feedback! The exact reason for the configuration error can be found in the log file of the runtime, see |
Thank you for your prompt reply! I might not be able to try again until next week, but I will get back to you then. |
I made some quick checks, and I can verify that the cookie problem was resolved by configuration settings. Are these configuration additions backward "compatible" (i.e. they don't break previous versions for any platform)? The I'll be able to post the full log file on Monday if you still need that. A fourth question, is there a setting to not show the large left menu after SEB is launched (it covers a third of the screen and requires a click-away to close)? |
Edit: I just realized the HEAD-issue is probably because I don't set any CORS headers. I'll try to change that on Monday. |
Aha, I see. Well, for performance reasons I would prefer to keep the mechanism as it is now, that particular method only checks whether the loader can access the resource (with a GET request, the resource would then be downloaded twice). Regarding the large left menu: This is a new feature of SEB 3.x called the Action Center (or side menu) and can be controlled in the "User Interface" tab. |
That sounds reasonable, setting CORS-headers and allowing HEAD requests are not an issue. Thanks, have a nice weekend and stay safe! |
Thanks, same to you. Let me know whether it worked out regarding the HEAD issue when you have the time to investigate it. |
It does seem like this Google storage unfortunately does not support HEAD-requests, I've found other reports of similar problems. We might be able to work around this, but I'd like to suggest skipping the HEAD-request and making a GET-request instead and also caching the downloaded data such that no additional request need to be made when the actual configuration is loaded. It does sound unnecessary, both for the individual client and for school networks, to make two requests for every student, even though the first is only a HEAD-request without any body in the response. |
Hm, are you absolutely sure that it isn't a configuration issue with your server? HEAD requests should work just fine, this literally is their purpose (to retrieve meta information about a resource, in this case to check whether it exists). Caching the resource is out of the question, since I can right off the top of my head come up with a scenario where caching could cause serious issues (say a school uses the same SEB instance during a day for multiple exams and always serves the exam configurations from the same URL, even though they are different). I could implement a fallback GET request in case the HEAD fails, but that would be unnecessary overhead in all those cases where a resource really doesn't exist... |
I'm testing this using Google Firebase storage, and the HEAD problem seems to be reported: firebase/firebase-js-sdk#2623 The response sounds like it will be fixed in the future, but that could take time. Also, chances are there are other storage hosting providers (today or in the future) with the same issue. The caching I referred to was simply between the two, almost simultaneous, requests (checking file and loading configuration), not longer than that. |
Okay, I have implemented a fallback GET request in case HEAD fails. But I did this quite reluctantly, since I do not like having to fix issues of other libraries in my code... I would welcome it if you could let me know once the issue is resolved in this Google library, so that I can potentially remove the fallback. |
Please test the latest build and let me know whether it works. |
Thank you @dbuechel ! Another note – the url might not point to an actual file but rather at backend code generating the configuration on the fly. Sending two requests might cause the backend to generate the data twice, using more server resources than intended. That can definitely be solved in the backend, but I just wanted to point out another possible drawback with using multiple requests. |
Okay, great.
That would be the right place to cache, IMHO 😉 |
Perhaps you're right, but remember that two subsequent requests to a URL might hit two completely different servers under a load balancer, so in that case, a central cache store need to be involved, which is probably slower than generating the data again. I obviously don't have a lot of knowledge of your implementation, but currently I don't see why the |
The reason is that there are multiple use cases involved here, one of them is the following: In case a resource needs authentication (response code 401 "Unauthorized"), SEB should load the resource in the browser so that a user may authenticate themselves, whereupon SEB will download the resource and reconfigure itself. In this and the other use cases, I deemed it more efficient to use HEAD requests to determine the status of a resouce, as this again literally is the purpose of HEAD requests. |
I downloaded https://github.com/SafeExamBrowser/seb-win-refactoring/releases/download/3.0.0-beta/SEB_3.0.0.118_SetupBundle.exe but that does not seem to include these changes. I get the same error and there are no mentions of Is there another link I can use to get the latest build? |
Yes, please use the direct link to the build server: https://sebdev-let.ethz.ch/project/appveyor/seb-win-refactoring/build/job/u6p88j9miqra29dg/artifacts |
Thanks. It works!
I still want to point out that performing 2–3 identical requests (3 if HEAD fails) within a few milliseconds might affect performance in school networks where hundreds of students does this simultaneously. Caching on the server has no effect on that. |
Are you sure by the way that the HTTP-layer you're using does not already respect cache-headers, thus not sending the second request if the first responded with cache-headers? |
Uhm, isn't it the case that there only now might be 3 requests solely because your library does not correctly handle HEAD requests...? 😕 Regarding your other point, I don't think there is any caching in the .NET API I am using, I however did not investigate this in great detail. There is a ton of work involved in writing a feature-rich application like SEB single-handed, I simply don't have the capacity to test out every minute detail. If you'd like to know it exactly, feel free to debug / test it yourself. I certainly aspire to improve the code whenever and wherever I can... |
If there aren't any other issues, I suggest we close this one for now? |
I'll take a look at the caching when I get the chance. It should be easy to test considering that there are two identical requests now if I reject the initial HEAD.
Absolutely, closed. |
Setting cache-headers on the response did not stop SEB from making additional identical requests. |
Great, thanks for investigating! |
During initial testing of SEB 3.0, I noticed three things. I don't know if these are known issues:
1) Using Microsoft Edge,sebs://
links don't trigger a start of SEB (nothing happens). Using Chrome, SEB is launched.2) Using a
sebs://
-link from Chrome starts SEB but triggers a "Configuration Error" message saying the resource is not supported, and SEB is quit. If I download the exact contents of the same URL and save it as an.seb
-file, it all works as expected, so there's nothing wrong with the actual configuration.Let me know if these are not known issues and if you want me to post a link that behaves in this way for me.
The text was updated successfully, but these errors were encountered: