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

Retries on 201 Created HTTP status code #208

Closed
donut opened this issue Mar 4, 2015 · 11 comments
Closed

Retries on 201 Created HTTP status code #208

donut opened this issue Mar 4, 2015 · 11 comments

Comments

@donut
Copy link

donut commented Mar 4, 2015

Resumable.js retries uploading a chunk when it receives a 201 Created response instead of continuing to the next chunk. If anything, 201 should be a more appropriate response for the successful uploading of a chunk than 200, right? Am I crazy and missing the logic here?

@steffentchr
Copy link
Member

Hi @donut,

This status codes used by default are mainly chosen based on what created notices in browser consoles. You can see the discussion here: #160

Having said that, you can configure the response to error codes when setting up resumable (and yes 201 would be reasonable as an ok status)

@donut
Copy link
Author

donut commented Mar 4, 2015

I see where I can set permanent error codes but I don't see where I can change the action taken when receiving 201. Any pointers?

@donut
Copy link
Author

donut commented Mar 4, 2015

The reason this is a pain is that the API is setup to take chunks from locations other than the web and I'm trying to avoid making all these exceptions for Resumable. Another pain point is not being able to customize the URL on a per-chunk basis and the inability to customize the default query parameter names. That said, I am really appreciating the ease of use that Resumable provides.

@steffentchr
Copy link
Member

You're right that it isn't easy to white-list replies -- you could send a pull request that make change in and around this line.
Are you looking to add different targets for different files or for individual chunks? I can see some use case, but wondering what you have in mind?

@donut
Copy link
Author

donut commented Mar 5, 2015

The API I have setup has an endpoint like /videos/{video_id}/chunks/{chunk_number}. It expects GET requests to have the chunk number as part of the endpoint. I tried using the preprocess setting to change the target option before each chunk is uploaded, but that caused problems (if I remember correctly, Resumable would never move on to actually upload the chunk). I ended up adding an exception to the API to also accept resumableChunkNumber query parameter.

I've already added the exception to send a 200 OK code when Resumable is the client. If I wasn't on such a tight deadline I'd love to do the pull request. I'll add it to my list. It looks like it'd be pretty easy to do.

Thanks for the responses!

@steffentchr
Copy link
Member

A good approach to that would be to allow the target option to be a function. If that's the case, then the function is expected to return the chunk for the URL for the chunk. This would updated getTarget with something like this:

getTarget:function(params){
  var target = $.getOpt('target');
  if(typeof(target)=='function') {
    return target(params);
  } else {
    if(target.indexOf('?') < 0) {
      target += '?';
    } else {
      target += '&';
    }
    return target + params.join('&');
  }
}

@steffentchr
Copy link
Member

(I would like to add that I have a special interest in video services, upload apis and that kind of stuff -- considering that it's my day job and the reason Resumable was created. So if you want to and can share more about what you're building I'm very interested.)

@donut
Copy link
Author

donut commented Mar 5, 2015

Sent you an email.

@axschech
Copy link

@steffentchr were any changes ever made here? I'm experiencing the same issue.

@steffentchr
Copy link
Member

@axschech I never made a change based on this, but if you're looking into it please submit a pull request and I'm happy to bring it into core.

@sreuter
Copy link

sreuter commented Jul 14, 2015

@axschech @donut Just a quick heads up. Everything discussed here should be addressed by now. See #231 and #233 ..

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

4 participants