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

Response status checking #3

Open
KKSzymanowski opened this issue Jan 1, 2017 · 3 comments
Open

Response status checking #3

KKSzymanowski opened this issue Jan 1, 2017 · 3 comments

Comments

@KKSzymanowski
Copy link

I have a concern about this piece of code:

axios[requestType](url, this.data())
    .then(response => {
        this.onSuccess(response.data);

        resolve(response.data);
    })
    .catch(error => {
        this.onFail(error.response.data);

        reject(error.response.data);
    });

Shouldn't you check if error.response.status is 422 before you record errors?
If the server returns eg. 403 or 500, the Errors::errors would contain the whole HTML response which might cause some fatal errors.

@simondavies
Copy link

simondavies commented Jan 5, 2017

you could try doing the following

    axios[requestType](url, this.data())
        .then(response => {
            this.onSuccess(response.data);
            resolve(response.data);
        })
        .catch(error => {
            if(error.response.status === 422) {
                this.onFail(error.response.data);
            }
            reject({status : error.response.status, message : error.response.statusText});
        });

it will detect the 422 and if not will then just respond with a simple error message rather than the whole page, might be a start?

@simondavies
Copy link

I have actually added to it to catch a bit better error, when dealing with mainly laravel.

axios[requestType](url, this.data())
    .then(response => {
        this.onSuccess(response.data);
        resolve(response.data);
    })
    .catch(error => {
        if(error.response.status === 422) {
            this.onFail(error.response.data);
        }
        if(error.response.data.message !== undefined) {
            reject({status : error.response.data.status, message : error.response.data.message});
        } else {
            reject({status : error.response.status, message : error.response.statusText});
        }
    });

Initially, it would catch the 422 then supply the error with the response.

I then try to catch two types of errors that are returned which I manage in my laravel php class something like:

    return response()->json([
        'status' => $response['status'],
        'message' =>  ($this->errors[$response['title']])?:'Unreported Error'
    ],$response['status']);

    return $response;

But again this might be as it's tailored for me and my app

As this might be for non laravel based apps you might need to, let the user actually, add their own method here themselves, as this is a great starter that you can add to yourself. But thought i mentioned what i have done, anyway

@codexp
Copy link

codexp commented Jun 17, 2017

here is my approach:

/**
 * Submit the form.
 *
 * @param {string} requestType
 * @param {string} url
 */
submit(requestType, url) {
    return new Promise((resolve, reject) => {
        axios[requestType](url, this.data())
            .then(({data}) => {
                resolve(data);
            })
            .catch(error => {
                this.onFail(
                    error.response.data,
                    error.response.status,
                    error.response.statusText
                );

                reject(error);
            });
    });
}

/**
 * Handle a failed form submission.
 *
 * @param {object} errors
 * @param {number} status
 * @param {string} statusText
 */
onFail(errors, status, statusText) {

    if (status === 422) {
        this.errors.record(errors);
    } else {
        this.errors.record({
            general: {
                0: status + ' ' + statusText
            }
        });
    }

    return this;
}

btw, I have removed onSuccess() since it has been doing not much.
And the reject() passes original error response (so you can check status in your app too).

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

3 participants