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

Cancellation may not work well with post #4338

Open
zhangzhiqiangcs opened this issue Dec 18, 2021 · 19 comments
Open

Cancellation may not work well with post #4338

zhangzhiqiangcs opened this issue Dec 18, 2021 · 19 comments

Comments

@zhangzhiqiangcs
Copy link

Describe the bug

Followed official doc, Use cancelToken to close pending reqeust, the browser side do cancel the request, but the server side did not receive request cancel signal.

But the get method works well as expected.

To Reproduce

Here is my demo code to reproduce the environment.

First start the server, go run main.go. Second run the client, node index.js.

When js client exit, the server not receive signal as expect.

#!/usr/bin/env node

const { default: axios } = require('axios')

console.log('Hello!')
const source = axios.CancelToken.source()
async function main() {
    axios
        .post('http://127.0.0.1:5978/debug/context', {}, { cancelToken: source.token })
        .then((resp) => {
            console.log(resp)
        })
        .catch((e) => {
            console.log('errs', e)
        })
    await new Promise((r) => setTimeout(r, 5000))
    console.log('cancel')
    source.cancel()
}

main()

Go Server

package main

import (
	"context"
	"fmt"
	"net/http"
	"time"

	"github.com/gorilla/mux"
)

func main() {
	router := mux.NewRouter()
	router.HandleFunc("/debug/context", func(w http.ResponseWriter, r *http.Request) {
		ctx := r.Context()
		handle(ctx)
		fmt.Fprint(w, "ok")
	}).Methods("POST")
	srv := &http.Server{
		Handler: router,
		Addr:    "0.0.0.0:5978",
	}
	panic(srv.ListenAndServe())
}

func handle(ctx context.Context) {
	exit := make(chan struct{})
	i := 1
	go func() {
	loop:
		for {
			time.Sleep(time.Second * 1)
			fmt.Println("handle func running", i)
			i++
			select {
			case <-exit:
				fmt.Println("subroutine exit")
				break loop
			default:
			}
		}
	}()
	<-ctx.Done()
	fmt.Println("ctx is done")
	fmt.Println("kill sub goroutine")
	exit <- struct{}{}
}

Expected behavior

when post request is canceld, expect server side recevice signal. For example, goroutine context is Done and release some resource.

Environment

  • Axios Version 0.24.0
  • Adapter HTTP
  • Browser Chrome, Nodejs
  • Browser Version Chrome: 96.0.4664.93
  • Node.js Version v14.17.6
  • OS: Ubuntu 18.04
  • Additional Library Versions: NULL

Additional context/Screenshots

Add any other context about the problem here. If applicable, add screenshots to help explain.

@arthurfiorette
Copy link
Contributor

Not saying that your/axios code has problems, but the CancelToken api is deprecated. Have you tried with the new one?

@zhangzhiqiangcs
Copy link
Author

@arthurfiorette thanks for you reply. I use the AbortController to test, the result is same.
here is demo code

#!/usr/bin/env node

const { default: axios } = require('axios')

console.log('Hello!')
const source = axios.CancelToken.source()
const controller = new AbortController()
async function main() {
    axios
        .post('http://127.0.0.1:5978/debug/context', {}, { signal: controller.signal })
        .then((resp) => {
            console.log(resp)
        })
        .catch((e) => {
            console.log('errs', e)
        })
    await new Promise((r) => setTimeout(r, 5000))
    console.log('cancel')
    controller.abort()
}

main()

@arthurfiorette
Copy link
Contributor

I may be misunderstanding what you are trying to do, but this property doesn't do it for you?

timeout?: number;

Anyways, can you provide your code output? Because i tried it with a 5 seconds request delay and i got it right (?)

# my output
> 'main()'
> 'Hello!'
> 'cancel'
> 'errs { message: "canceled" }'

@zhangzhiqiangcs
Copy link
Author

@arthurfiorette timeout may not work work perfectly for me. I use axios to send a post request, I do not actually know how much time it may consume, but I want to stop the request when user leave this page, because the request locked some resource on the server side. (Maybe I have a bad application design)

The demo code sleep 5 seconds just to simulate the request pending time. My output is the same as yours.
here

Hello!
cancel
errs Cancel { message: 'canceled' }

But this problem is only happened to post method, when change to get method, the server side can knows the request is finished. You can check get method by the following code
Go Server

package main

import (
	"context"
	"fmt"
	"net/http"
	"time"

	"github.com/gorilla/mux"
)

func main() {
	router := mux.NewRouter()
	router.HandleFunc("/debug/context", func(w http.ResponseWriter, r *http.Request) {
		ctx := r.Context()
		handle(ctx)
		fmt.Fprint(w, "ok")
	}).Methods("GET")
	srv := &http.Server{
		Handler: router,
		Addr:    "0.0.0.0:5978",
	}
	panic(srv.ListenAndServe())
}

func handle(ctx context.Context) {
	exit := make(chan struct{})
	i := 1
	go func() {
	loop:
		for {
			time.Sleep(time.Second * 1)
			fmt.Println("handle func running", i)
			i++
			select {
			case <-exit:
				fmt.Println("subroutine exit")
				break loop
			default:
			}
		}
	}()
	<-ctx.Done()
	fmt.Println("ctx is done")
	fmt.Println("kill sub goroutine")
	exit <- struct{}{}
}

Client

#!/usr/bin/env node

const { default: axios } = require('axios')

console.log('Hello!')
const source = axios.CancelToken.source()
const controller = new AbortController()
async function main() {
    axios
        .get('http://127.0.0.1:5978/debug/context', { signal: controller.signal })
        .then((resp) => {
            console.log(resp)
        })
        .catch((e) => {
            console.log('errs', e)
        })
    await new Promise((r) => setTimeout(r, 5000))
    console.log('cancel')
    controller.abort()
}

main()

thanks again.

@DigitalBrainJS
Copy link
Collaborator

@zhangzhiqiangcs It looks like this is some kind of problem with your backend (proxy/server). You can simply use your network console to make sure that the Axios connection is aborted normally.

node.js server:

const Koa = require("cp-koa");
const serve = require("koa-static");
const Router = require("koa-router");
const {CPromise} = require("c-promise2");

const app = new Koa();
const router = new Router();

router.all("/api", async (ctx, next) => {
  const ts = Date.now();
  console.log(`\nRequest [${ctx.url}]`);

  await ctx.run(function* () {
    this.onCancel(() => {
      console.warn(`Request aborted after [${Date.now() - ts}ms]`)
    });
    yield CPromise.delay(3000);
    ctx.body = `Hello! ${Date.now()}`;
    console.log(`Done [${ctx.url}] [${Date.now() - ts}ms]`);
  });
});

app
  .use(router.routes())
  .use(router.allowedMethods())
  .use(serve("./public"))
  .listen(8080);

client page (./public/index.html):

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8" />
  <title>Title</title>
  <script src="https://cdn.jsdelivr.net/npm/axios/dist/axios.min.js"></script>
</head>
<body>
<button onclick="doRequest(this, true)">POST with cancellation</button>
<button onclick="doRequest(this)">POST</button>
<script>
  async function doRequest(button, cancel= false) {
    button.innerText = "Requesting...";
    const controller = new AbortController();
    axios
      .post("/api", {time: Date.now()}, {
        signal: controller.signal
      })
      .then((resp) => {
        button.innerText = `Done: ${JSON.stringify(resp.data)}`;
        console.log(resp);
      })
      .catch((e) => {
        button.innerText = `Fail: ${e}`;
        console.log("errs", e);
      });
    if(cancel) {
      await new Promise((r) => setTimeout(r, 1000));
      console.log("cancel");
      controller.abort();
    }
  }
</script>
</body>
</html>

normal request log:

Request [/api]
Done [/api] [3016ms]

aborted request log:

Request [/api]
Request aborted after [1206ms]

@zhangzhiqiangcs
Copy link
Author

@DigitalBrainJS thanks for your explanation. You are right, I realized that this is a server/backbend problem.

Maybe It is relevant to server's http implementation, I will dig into golang internal or turn to community to find the exact answer.

I will close this issue. @arthurfiorette thanks you again too.

@zhangzhiqiangcs
Copy link
Author

@DigitalBrainJS @arthurfiorette Sorry for being garrulous. But I notice that when using curl command to post, and then using ctrl + c to cancel request, the server can catch the end of the request.

I wonder whether it is relevant with axios Cancellation implemention ?

@gerryfletch
Copy link

I'm also finding that the AbortController isn't cancelling the request. It's putting the promise into a failed state, but the upload appears to continue as before.

@alienzhangyw
Copy link

I'm upgrading CancelToken to AbortController, but something goes wrong.
I use AbortController as a global variable,and set axios defaults like this:

window.abortController = new AbortController();
axios.defaults.timeout = 30000;
axios.defaults.signal = window.abortController.signal;

And cancel some requests when user is closing a modal:

  const closeModal = () => {
    dispatch(toggleHisModal(false));
    if (window.abortController) {
      window.abortController.abort();
    }
  };

After one single abortion, all of the requests can't be made again, yes all of them. Nothing happens in the F12 network panel. But those request promises are likely resolved directly and returned null, so that I get errors in 'then' handler instead of 'catch'.

Did I use AbortController in a wrong way?

@grandsilence
Copy link

@alienzhangyw I have the same issue but even in local scope. After running some tests on a perfectly working .NET 6 WebAPI Server with request cancellation support, I found out that Axios really doesn't cancel requests, it just putting the promise into a failed state as @gerryfletch said.

Again, the server is absolutely correct in canceling requests, I checked this in Postman.

Surprisingly, using build-in fetch(url, { signal }), the request isn't cancelled either in Google Chrome or in Firefox browsers! Promise changed state to failed but the request is fully processed by server without cancellation.

@arthurfiorette
Copy link
Contributor

arthurfiorette commented May 17, 2022

Interesting, could that be something framework specific with your server implementations?

Can you create a simple nodejs/python server to test this idea?

If even fetch didn't work, could this issue be out of scope for the xhr axios adapter?

@grandsilence
Copy link

grandsilence commented May 17, 2022

UPDATED

FOUND THE REASON for me:
I used proxy server for my React SPA (http-proxy-middleware package in setupProxy.js) and it breaks cancellation behaviour. For production mode and original server everything working as expected.

API Proxy (SPA) always handle requests to origin server (cancellation isn't supported in http-proxy-middleware package).


@arthurfiorette If you need a demo WebAPI project for C# (.NET 6), I can make a new demo repo.

We will also need to check how it handles via deprecated axios.CancelToken?


Here is basic NodeJS server (express) with cancellation support:

const express = require('express');
const app = express();
const port = 8080;

app.get('/', (req, res) => {
	let executed = false;

	// any resource-intensive task can be simulated here
	const timeout = setTimeout(() => {
		res.end('OK!')
		console.log('[OK] Response completely handled.');
		executed = true;
	}, 3000);

	// on success or cancelled (close connection)
	req.on('close', () => {
		// you can also use AbortController and AbortController.signal
		// for cancel handling: https://leanylabs.com/blog/cancel-promise-abortcontroller/
		if (!executed) {
			clearInterval(timeout);
			console.log('[CANCELLED] Request cancelled.');
		} else {
			console.log('[DONE] Request Closed');
		}
	});
});

app.listen(port, () => {
  console.log('Example app listening on port: ' + port);
});

I'm using NodeJS v16.15.0 for now on Windows 10 1803 OS. Latest browsers (Chrome and Firefox).

@arthurfiorette
Copy link
Contributor

Glad to hear that!!!

@zhangzhiqiangcs, any chance of you confirming if there's some kind of proxy messing with cancelation too?

@zhangzhiqiangcs
Copy link
Author

@arthurfiorette Actually, I didn't use proxy in my front-end demo code. I check it again here

#!/usr/bin/env node

const { default: axios } = require('axios')

console.log('Hello!')
const source = axios.CancelToken.source()
const controller = new AbortController()
async function main() {
    axios
        .post('http://127.0.0.1:5978/debug/context', {}, { signal: controller.signal })
        .then((resp) => {})
        .catch((e) => {
            console.log('errs', e)
        })
    await new Promise((r) => setTimeout(r, 5000))
    console.log('cancel')
    controller.abort()
}

main()

I typeed node index.js to execute it. And node version is v16.13.0

@alienzhangyw
Copy link

FOUND THE REASON for me: I used proxy server for my React SPA (http-proxy-middleware package in setupProxy.js) and it breaks cancellation behaviour. For production mode and original server everything working as expected.

So if it's a proxy problem, how can we deal with it when using webpack-dev-server in React development mode?

@grandsilence
Copy link

grandsilence commented May 17, 2022

@alienzhangyw

UPDATED

Clone demo project

You can clone the demo repository: react-demo-abort-controller-proxy-issue.

git clone https://github.com/grandsilence/react-demo-abort-controller-proxy-issue.git
cd react-demo-abort-controller-proxy-issue
npm install
npm run server
npm run start

Or create new project

  1. Create new React app using Create React App:
    npx create-react-app my-app
  2. Create src/setupProxy.js file with content:
    const { createProxyMiddleware } = require('http-proxy-middleware');
    
    module.exports = function (app) {
      app.use(
        '/api',
        createProxyMiddleware({
          target: 'http://localhost:8080',
          changeOrigin: true,
          secure: false,
        }),
      );
    };
  3. In my express demo server you should change endpoint path from / to /api/test (for proxy request matching):
    app.get('/api/test', (req, res) => { // ...
  4. Edit App.js:
    import { useState } from 'react';
    import axios from 'axios';
    
    function App() {
      const [response, setResponse] = useState(null);
      const [abortControllers, setAbortControllers] = useState([]);
      const apiUrl = '/api/test';
    
      const axiosRequest = () => {
        const abortController = new AbortController();
        return [
          axios.get(apiUrl, { signal: abortController.signal }).then((res) => res.data),
          abortController,
        ];
      };
    
      const fetchRequest = () => {
        const abortController = new AbortController();
        return [
          fetch(apiUrl, { signal: abortController.signal }).then((result) => result.text()),
          abortController,
        ];
      };
    
      const handleRequest = (requestMethod) => {
        setResponse(null);
        const [requestPromise, abortController] = requestMethod();
        requestPromise
          .then((result) => {
            setResponse(result);
          })
          .catch((e) => console.log(e));
    
        setAbortControllers((old) => [abortController, ...old]);
      };
    
      const handleCancelAll = () => {
        abortControllers.forEach((abortController) => {
          abortController.abort();
        });
      };
    
      return (
        <>
          <div>{response}</div>
          <div>
            <button onClick={() => handleRequest(axiosRequest)}>Axios Fetch +1</button>
            <button onClick={() => handleRequest(fetchRequest)}>Native Fetch +1</button>
          </div>
          <button onClick={handleCancelAll}>Cancel All</button>
        </>
      );
    }
    
    export default App;
  5. Start Demo server: node index.js:
    const express = require('express');
    const app = express();
    const port = 8080;
    
    app.get('/api/test', (req, res) => {
        let executed = false;
    
        // any resource-intensive task can be simulated here
        const timeout = setTimeout(() => {
    	    res.end('OK!')
    	    console.log('[OK] Response completely handled.');
    	    executed = true;
        }, 3000);
    
        // on success or cancelled (close connection)
        req.on('close', () => {
    	    // you can also use AbortController and AbortController.signal
    	    // for cancel handling: https://leanylabs.com/blog/cancel-promise-abortcontroller/
    	    if (!executed) {
    		    clearInterval(timeout);
    		    console.log('[CANCELLED] Request cancelled.');
    	    } else {
    		    console.log('[DONE] Request Closed');
    	    }
        });
    });
    
    app.listen(port, () => {
      console.log('Example app listening on port: ' + port);
    });
  6. Start React project in development mode: npm run start

Description of the resulting behavior

After you can make some requests, cancel all and reproduce the issue.
You will get messages on the server (after dev proxy):

[OK] Response completely handled.
[DONE] Request Closed

But expected demo server behavior is:

[CANCELLED] Request cancelled.

@gerazenobi
Copy link

Hi there 👋

I have a use case where I was triggering multiple POST requests and trying to abort them using only 1 controller/abort signal: it didn't work. I simplified it to troubleshoot the issue to just 1 POST request, and it didn't work either: the request goes through and the promise is resolved normally.

I don't think it is a server problem as I am hitting an Elastic Search API.

I am using the axios client instance ( axios.create(...))

this.client = axios.create(...);
// the function that triggers the request:
this.requestsController = new AbortController();
const abortSignal = this.requestsController.signal;
this.results = await this.client.post(`/search.json`, payload, { signal: abortSignal });

then in a click handler for a clear search button I have the abort like so:

if (this.requestsController) {
    this.requestsController.abort();
}

I am using axios @0.21.1

Any help would be much appreciated 🙏

@ysela
Copy link

ysela commented Feb 6, 2023

Hi there 👋

I have a use case where I was triggering multiple POST requests and trying to abort them using only 1 controller/abort signal: it didn't work. I simplified it to troubleshoot the issue to just 1 POST request, and it didn't work either: the request goes through and the promise is resolved normally.

I don't think it is a server problem as I am hitting an Elastic Search API.

I am using the axios client instance ( axios.create(...))

this.client = axios.create(...);
// the function that triggers the request:
this.requestsController = new AbortController();
const abortSignal = this.requestsController.signal;
this.results = await this.client.post(`/search.json`, payload, { signal: abortSignal });

then in a click handler for a clear search button I have the abort like so:

if (this.requestsController) {
    this.requestsController.abort();
}

I am using axios @0.21.1

Any help would be much appreciated 🙏

According to the Axios docs, Abort Controller support begins with version 0.22.0, so your best bet would be to upgrade

@Apollon76
Copy link

I have two clients for a single http server: one written in kotlin using OkHttp and another written in typescript using axios. For axios I'm using AbortController but it doesn't cancel requests on a server side. OkHttp cancels requests normally.

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

10 participants