try/catch block does not work here

[Lukezzz]

Hi,

I have a code where I upload a file to a server. The code is shown below.
As seen I use a loop to try to connect 10 times if attempts is failing.

I have also put this in a try/catch block and at the end, I am showing a messagebox with confirmation that the file was created on the server.

The problem is this now. Sometimes the file is created and sometimes the file is Not created.
When the file is Not created, the messageBox is shown anyway. This is what I dont understand and that is the problem.

If the code have reached the messageBox it means that the above code has succeded but that is not the case, the file is not created on the serverpath.

How can this happen and can be done ?

Thank you...

for( int i = 0; i < 10; i++ ) //try 10 times
 {
 Thread::Sleep(200);

try
{
	String^ LongString = "stringWithText";
	Chilkat::SFtp^ sftp = gcnew Chilkat::SFtp();
	sftp->UnlockComponent("dummy");
	
	sftp->ConnectTimeoutMs = 5000; //Set some timeouts, in milliseconds:
	sftp->IdleTimeoutMs = 15000;

	int port = 2222;
	String^ hostname = "dummy.hostname.com";
	sftp->Connect(hostname, port);
	sftp->AuthenticatePw("userName","passWord");
	sftp->InitializeSftp(); //After authenticating, the SFTP subsystem must be initialized:


	String^ handle1 = sftp->OpenFile("Folder1/Folder2/testFile.txt", "writeOnly", "createTruncate");
	sftp->WriteFileText(handle1,"ansi", LongString); //Write some text to the file:
	sftp->CloseHandle(handle1);
	MessageBox::Show("Successfully sent", "Info ", MessageBoxButtons::OK, MessageBoxIcon::Information);

	Application::DoEvents();
	this->Close();
	break;
		
}
catch(Exception^ ex){}
}

[thelamb]

Are you sure this 'Chilkat' FTP library throws exceptions when something goes wrong? Maybe it is expecting you to check the return value of WriteFile etc.

[Lukezzz]

thelamb, you are right, I am expected to check the return value. Actually I have not needed to do that until now, so I am not sure of how to check that.

I got this answer from them but have not got any other answer in over 2 days, so I might ask here then.

(Their answer)
>> You should be checking the return value of each call to a Chilkat method
to see whether it succeeded or not.
If it did not, then you should not proceed with the remainder. For
example, if
sftp->Connect(hostname, port);
returns false, then don't continue...

So as an example. How to check if return value is true or false here ?:

sftp->Connect(hostname, port);

[JasonHippy]

Have you tried this??

if(sftp->Connect(hostname, port))
{
	// do something
}
else
{
	// Whoops sftp->Connect() failed!!
}

Cheers for now,
Jas.

[Lukezzz]

No I have not tried that.. I am not sure how to do it.
Without testing,

Are you sure that your example is correct there ?
I just have to be sure, because if I test that code and if it works I dont know if it is depending on that as my code sometimes work but it seems logical ?

[Fbody]

One nice thing about C++ is the way it interprets true and false. There are, of course, the keywords true and false, but you can also use zero vs. non-zero. Zero is false, and non-zero is true.

All you need is:

if (sftp->Connect(hostname, port)) {
  //perform your transfer if true/non-zero
} else {
  //error handling if false or zero
}

edit:
wow, double ninja, I'm slow...

[Lukezzz]

I understand that sounds wonderful. I have to ask about one detail though of how to set up this parameter. Do I need to first connect and then check this in the if statment or will I just write the if statement.

For example do I need the first line here ?

sftp->Connect(hostname, port);

if (sftp->Connect(hostname, port)) {
  //perform your transfer if true/non-zero
} else {
  //error handling if false or zero
}

[JasonHippy]

I understand that sounds wonderful. I have to ask about one detail though of how to set up this parameter. Do I need to first connect and then check this in the if statment or will I just write the if statement.

For example do I need the first line here ?

sftp->Connect(hostname, port);

if (sftp->Connect(hostname, port)) {
  //perform your transfer if true/non-zero
} else {
  //error handling if false or zero
}

{facepalm}

No, you don't need to call it twice! You just call it once inside the if statement as recommended by myself and Fbody!

Basically the Connect function is called inside the if statement. The if statement then evaluates the return value from the Connect call.
So if the Connect function returns true (or non zero), then the code under the if statement is executed. Otherwise if Connect returns false (or 0) then the code under the else is used!

I hope that clears things up for you!

[Lukezzz]

Thats wonderful, I have not really thought about in that way, then I feel comfortable to check it like that as for other parameters.

Thanks alot for your help!

[Lukezzz]

I did notice that this code does not work anyway.

The first time, it did upload the file to the server and showed the messagebox that it succed to create the file. That was correct.

I deleted the file on the server and runned the code again. I received the messagebox that the file was created succesfully but the file was not created this time.

My idéa here was the check the very last thing that happens, where I close the file.
I think this must be enough to check this as if the file was not created there is no file to close ?

So if closing the file returns true then I did assume that everything above went well:
if( sftp->CloseHandle(handle1) )

So I then wonder how this is possible ?

int succeded = 0;
try
{
		String^ LongString = "textString";
		Chilkat::SFtp^ sftp = gcnew Chilkat::SFtp();
		sftp->UnlockComponent("dummy");
		
		sftp->ConnectTimeoutMs = 5000; //  Set some timeouts, in milliseconds:
		sftp->IdleTimeoutMs = 15000;

		int port = 2222;
		String^ hostname = "dummy.host.com";
		sftp->Connect(hostname, port);
		sftp->AuthenticatePw(tr1,tr2);
		sftp->InitializeSftp(); //  After authenticating, the SFTP subsystem must be initialized:


		String^ handle1 = sftp->OpenFile("Folder1/Folder2/testFile.txt", "writeOnly", "createTruncate");
		sftp->WriteFileText(handle1,"ansi", LongString); //  Write some text to the file:
		
		if( sftp->CloseHandle(handle1) )
		{
			succeded = 1;
			MessageBox::Show("File successfully created!", "Info ", MessageBoxButtons::OK, MessageBoxIcon::Information);

			Application::DoEvents();
			can_exit = true;
			this->Close();
			break;
		}
		
}
catch(Exception^ ex){}


if( succeded == 0 )
{
	MessageBox::Show("File not created", "Info ", MessageBoxButtons::OK, MessageBoxIcon::Information);
}

[thelamb]

Not necessarily, everything you are assuming here depends on implementation details of the library.
It is unsafe, and bad practice to do this.

Why not check every function (provided that it returns something that indicates an error of course)?

You can still use the try/catch block to make life a little easier:

try {
    if( ! sftp->Connect(hostname, port) )
        throw std::exception( "Connect failed." );

} catch( std::exception& except )
{
    cout << "Exception! Message: " << except.what() << "\n";
}

*edit: cout is ofcourse only as an example.. if you don't have a console application this won't work out of course*

Then you should look at the libraries documentation if they provide any error-code that will give you more details about _what_ exactly went wrong.

[JasonHippy]

If it worked once, why shouldn't it work again??

Did you refresh the folder view on the server after it claimed to have written the file the second time? (you probably did, but I mention it on the off-chance that you didn't!)

I also notice that you are not checking whether OpenFile or WriteFileText succeeded...Would it not be an idea to test the success or failure of either of those?

If the file wouldn't open in the first place, that would be the first potential point of failure. Attempting to write to a file that wasn't actually open would be a segfault in the making if you ask me! CRASH!! heh heh!

Other than that the code looks more or less OK.

[Lukezzz]

Yes you are right, I will have to check every statement and see if that succeds.
I have checked their documentation and I can put a boolean to see if things will be true or false and then show a messagebox with the error.

In their documentation they are writing "const char * hostname" and "const char * handle1"

I beleive that is for C# ? Because when I compile that I receive errors:
"cannot convert parameter 1 from 'const char *' to 'System:: String ^'"
cannot convert from 'System:: String ^' to 'const char *'

I beleive I can write: String^ hostname;
But I am more confused of const char * handle1 as the comparison below is:
if ( handle1 == 0 )

bool success = false;
								
long port = 2222;
const char * hostname = "dummy.host.com";
								
success = sftp->Connect(hostname, port);
if( success != true ){MessageBox::Show(sftp->LastErrorText->ToString());}//Check the error


const char * handle1;
handle1 = sftp->OpenFile("Folder1/Folder2/testFile1.txt", "writeOnly", "createTruncate");
if( handle1 == 0 ){MessageBox::Show(sftp->LastErrorText->ToString());}//Check the error

[Fbody]

That success variable is completely unnecessary. It wastes memory and processing cycles. An optimizing compiler may smooth it over, but I doubt it...
You should compress that to (like we've been telling you):

long port = 2222;
const char * hostname = "dummy.host.com";

if (!sftp->Connect(hostname, port)){MessageBox::Show(sftp->LastErrorText->ToString());}
//attempt connect and check for error

I don't know how your library works, but this seems wrong to me:

const char * handle1;
handle1 = sftp->OpenFile("Folder1/Folder2/testFile1.txt", "writeOnly", "createTruncate");
if( handle1 == 0 ){MessageBox::Show(sftp->LastErrorText->ToString());}//Check the error

According to your documentation, what are the return type and value from OpenFile()? I suspect that handle should be a pointer to some sort of integer type...

[Lukezzz]

Yes I will go back and put all statments as you said from the beginning.
I just get confused as in their documenation they use bool succes in the way I showed here.

I dont know what return type the OpenFile() returns but the 1 page documentation for the whole scenario is here if you want to see. I just cant understand how if( handle1 == 0 ) in their documentation, as you say I also looks for an integer type from somewhere but cant find it.
http://www.example-code.com/mfc/sftp_writeUtf8.asp

I will put it like you said from the beginning now and check everything and I will tell what this leads me soon.

[Fbody]

I think you need to look at that example more closely. The proper function name is openFile() NOT OpenFile(). C++ is case-sensitive, I think you need to pay closer attention to your casing. Because of the casing, openFile() and OpenFile() are not the same function name.

These are all valid, and more importantly, unique, function names. Each version will call a different function:
openfile()
openFile()
OpEnFiLe()
OPENFILE()
OpenFile()
etc...

[Lukezzz]

yes I did check that before also.

It is not possible to write:
sftp->openFile in my compiler... this will be a compileError.

The container only have "OpenFile" with a versal like that.

So still the if( handle == 0 ) is confusing as it later is expected as String^ in the first argument. (I am still trying with your approach)

[Fbody]

If that's the case, I would be concerned about the quality of the wrapper you are using. If it doesn't follow the library properly, its behavior will be unpredictable at best.

It appears that it is using a char* as an int* to be used as a handle for a file stream. Most likely, OpenFile() returns a NULL pointer on error. A NULL pointer is simply a pointer that is equal to 0 (doesn't point at anything/have a target). Post the declaration of OpenFile() from the proper header. I'm betting there is an inconsistency there.

[Lukezzz]

Yes the OpenFile() should be null there as you say. I had the messageBoxes after each declaration and had actually 4 error messages the FIRST loop, then the second loop I did not get any error messages and the file was created fine.
It is little spooky, I have sent these to the support of those functions to see if I miss something. The code in their documenation is for C# so something might perheps dont be the same for C++.

As seen I have put 30 cycles in that loop now and I tried to upload the file 10 times now and it did succed 10/10 times, which is a good indication.

If it doesn´t succeed, a messageBox will tell to try in a few seconds again. Perheps I will be happy with this, what do you think ?

int succeded = 0;
String^ LongString = "testString";

for( int i = 0; i < 30; i++ ) //try 10 times
 {
	 succeded = 0;
	 Thread::Sleep(5);

	try
	{
			for( int aaa = 0; aaa < 1; aaa++ )//just for break
			{									
				Chilkat::SFtp^ sftp = gcnew Chilkat::SFtp();
				
				if( ! sftp->UnlockComponent("dummy") ){succeded = 1; break;}//Check if succeded

				sftp->ConnectTimeoutMs = 5000; 
				sftp->IdleTimeoutMs = 15000;

				long port = 2222;
				String^ hostname = "dummy.host.com";
												
				if( ! sftp->Connect(hostname, port) ){succeded = 1; break;}//Check if succeded
				
				if( ! sftp->AuthenticatePw(tr1,tr2) ){succeded = 1; break;}//Check if succeded
				
				if( ! sftp->InitializeSftp() ){succeded = 1; break;}//Check if succeded


				String^ handle1;

				handle1 = sftp->OpenFile("Folder1/Folder2/testFile.txt", "writeOnly", "createTruncate");
				if( handle1 == nullptr ){succeded = 1; break;}//Check if succeded
												
				if( ! sftp->WriteFileText(handle1,"ansi", LongString) ){succeded = 1; break;}//Check if succeded
	
				if( ! sftp->CloseHandle(handle1) ){succeded = 1; break;}//Check if succeded
			}//to break

			if( succeded == 0 )//No Errors along the way down to here
			{
				MessageBox::Show("File Created!", "Info ", MessageBoxButtons::OK, MessageBoxIcon::Information);

				Application::DoEvents();
				can_exit = true;
				this->Close();
				break;
			}								
	}//try
	catch(Exception^ ex){}

 }//for loop 30 times

			if( succeded == 1 )
			{
				MessageBox::Show("File not created... Please try again in a few seconds", "Info ", MessageBoxButtons::OK, MessageBoxIcon::Error);
			}