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

Validation response error and general feedback #46

Closed
vijaymarupudi opened this issue Dec 14, 2022 · 10 comments
Closed

Validation response error and general feedback #46

vijaymarupudi opened this issue Dec 14, 2022 · 10 comments

Comments

@vijaymarupudi
Copy link

The general uploading a file process worked great! This is amazing! I think it'll be very intuitive for regular users with some UI/UX tweaks.

I wish the server prepended the timestamp to the filename, this would prevent malicious actors from overwriting existing data.

I ran into one bug. When validation is on, but the data does not conform to the specification, the server errors out instead of returning error information gracefully. In this case, the server creates a component but did not upload the data.

Here are the response headers in that case. Note that it also does not include the appropriate cross-origin headers.

HTTP/3 500 Internal Server Error
cache-control: private
content-encoding: gzip
content-type: text/plain; charset=utf-8
server: Google Frontend
x-cloud-trace-context: f98af38c066dd5a7141ef86c54ac6d29
x-content-type-options: nosniff
accept-ranges: bytes
date: Wed, 14 Dec 2022 22:33:39 GMT
x-served-by: cache-msp11872-MSP
x-cache: MISS
x-cache-hits: 0
x-timer: S1671057218.690842,VS0,VE1744
vary: x-fh-requested-host, accept-encoding
alt-svc: h3=":443";ma=86400,h3-29=":443";ma=86400,h3-27=":443";ma=86400

Additional info if needed:

I'm running this Javascript code from my personal domain:

fetch("https://pipe.jspsych.org/api/data/", {
  method: "POST",
  headers: {
    "Content-Type": "application/json",
    Accept: "*/*",
  },
  body: JSON.stringify({
    experimentID: "VbtDBjDAOin2",
    filename: "PID.json",
    data: '{"name": "Testing"}',
  }),
});

Since this is a cross-domain request, Firefox (or any browser I believe) sends an OPTIONS request and tests for the appropriate Access-Control-Allow-Origin header in the response.

Here were the appropriate OPTIONS response headers

HTTP/2 204 No Content
access-control-allow-headers: content-type
access-control-allow-methods: GET,HEAD,PUT,PATCH,POST,DELETE
access-control-allow-origin: https://vijaymarupudi.com
@vijaymarupudi
Copy link
Author

Correction: Looks like it does not allow you to overwrite, which is great!

@jodeleeuw
Copy link
Member

Thanks @vijaymarupudi this is super helpful!

I thought the server was properly configured for CORS requests. I'm using the cors package and it should be handling the OPTIONS header:

const corsHandler = cors({ origin: true });

The validation stuff is not well tested yet, so I wonder if there's just a bug in that code that caused the 500 response. I'll work on testing that more soon.

@jodeleeuw
Copy link
Member

Also

I think it'll be very intuitive for regular users with some UI/UX tweaks.

Any specific suggestions here? The main things left on my list are making it responsive to smaller screen sizes. I'd love to hear what is less intuitive at the moment so I can add that to the list.

@vijaymarupudi
Copy link
Author

I think the CORS failure if probably due to the lack of error handling for JS errors. But if the validation code is debugged, it should be fine.

The homepage could include the workflow of adding code to the experiment to make the data upload work. It's so simple that I think showing that immediately to people would make them more likely to use it and understand what to expect.

If people were going to use this to host an experiment, they would probably need to see a privacy policy specifically for DataPipe, so that they have something to show IRB folks that the data will not be used for any purposes and that it will be simpled passed through.

Providing information about OSF's storage limits https://help.osf.io/article/137-osf-storage-caps and privacy policy https://github.com/CenterForOpenScience/cos.io/blob/master/PRIVACY_POLICY.md. I'm not sure if OSF guarantees that they will not retain the files even after they are deleted.

This information could probably be consolidated to an IRB FAQ or privacy page.

It would be nice to be able to add conditions after an experiment has been created, but this might not be a high priority for many users.

The data validation settings should clarify that the JSON property is checked on every item in the array sent back, not the object sent back. Now that I think about it, this might explain the data validation bug, you should probably check that it is an array before you iterate over it.

Finally I ran into another bug, I enabled the session limit and now it shows me this error message, I can't seem to get back to the previous screens at all so I cannot test further :(:

Application error: a client-side exception has occurred (see the browser console for more information).

TypeError: e is null
    NextJS 33
        p
        z
        <anonymous>
        F
        ak
        ui
        i
        oD
        oO
        oO
        oE
        ox
        x
        T
        53
        c
        3840
        c
        4448
        c
        3935
        c
        745
        c
        4534
        c
        1783
        c
        <anonymous>
        O
        <anonymous>
        o
        <anonymous>
framework-3b5a00d5d7e8d93b.js:9:69816

@jodeleeuw
Copy link
Member

There's definitely a bug in the validation code. I'm using the joi library and experiencing a learning curve 😅. I'm gonna work on that first and see if it resolves other downstream issues...

@jodeleeuw
Copy link
Member

Finally I ran into another bug, I enabled the session limit and now it shows me this error message, I can't seem to get back to the previous screens at all so I cannot test further :(:

I'm having a hard time replicating this bug. Can you recall the steps you took to generate it? Was it on the new experiment page or the existing experiment page?

@vijaymarupudi
Copy link
Author

It was in the existing experiment page: I turned on the session limit (I believe I turned it off when creating the experiment)

@jodeleeuw
Copy link
Member

I think I identified this bug and fixed it (app is currently redeploying). It may not recover for the old experiment, but hopefully logging into the app again and creating a new experiment should work.

@vijaymarupudi
Copy link
Author

I can confirm that it now works for a new experiment! The old experiment probably will never work so I deleted it.

Sorry about not contributing code / bug fixes for the data validation part! Time has been hard to find lately and I don't know how to work with Firebase...

@jodeleeuw
Copy link
Member

No problem, this was super helpful!

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

No branches or pull requests

2 participants