COM error 0x80131500 on invoking issue selection with invalid parameters

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Install version 0.5 on Windows 7 x64 

What is the expected output? What do you see instead?
COM error occurs when clicking 'Select issue' button:

Failed to start the issue tracker COM provider 'Gurtle 0.5 (Google Code)'.
Unknown error 0x80131500

What version of the product are you using? On what operating system?
0.5
Windows 7 x64 Ultimate

Please provide any additional information below.

Original issue reported on code.google.com by kaistizen on 27 Sep 2009 at 7:09

[GoogleCodeExporter]

Sorry guys. 
After clicking 'Test' button in TortoiseSvn menu, it worked!
Even it is working now, still not working without clicking 'Test' button seems 
like a
potential problem. 

Original comment by kaistizen on 27 Sep 2009 at 7:12

[GoogleCodeExporter]

What 'test' button do you mean? I can't find such a button in the TSVN menu.

Original comment by tortoisesvn on 27 Sep 2009 at 8:27

[GoogleCodeExporter]

'Configure issue tracker integration' in the tortoisesvn settings -> Options -> 
Test

Original comment by kaistizen on 27 Sep 2009 at 8:30

[GoogleCodeExporter]

That's the options dialog of Gurtle. It's only started by TSVN but shown by 
Gurtle
itself.

But all Gurtle does when you click on that Test button is to download the page
http://projectname.google.com/
to see if the project exists. It doesn't do anything else.
Very strange that this would actually 'fix' your issue.

Original comment by tortoisesvn on 27 Sep 2009 at 9:04

[GoogleCodeExporter]

I was wrong. The reason why I saw the COM error was that the project name I 
specified 
had a big letter, for example Myproject instead of myproject. It seems that you 
get a 
COM error when your project name has a wrong capital.

Original comment by kaistizen on 27 Sep 2009 at 11:47

[GoogleCodeExporter]

It would be much better to make 'Test' mandatory or to make an error message 
more user-
friendly. Thank you for your response to my stupid question. ^^

Original comment by kaistizen on 27 Sep 2009 at 11:49

[GoogleCodeExporter]

> Failed to start the issue tracker COM provider 'Gurtle 0.5 (Google Code)'.
> Unknown error 0x80131500

I have been able to reproduce this error. It occurs when the parameters are 
invalid. 
As you noticed, a project name that is not all lowercase causes Gurtle to throw 
ParametersParseException with the message, "An error occurred parsing 
parameters." 
Unfortunately, TSVN shows a generic error message and the COM error code but 
not the 
error message returned by the plug-in. I have accepted this as a Gurtle defect 
nonetheless because in the even of other exceptoins later on, Gurtle does show 
its 
own error dialog box and it only seems consistent to do the same in this case 
too.

Original comment by azizatif on 27 Sep 2009 at 10:36

• Changed title: COM error 0x80131500 on invoking issue selection with invalid parameters
• Changed state: Accepted
• Added labels: Milestone-Release1.0

[GoogleCodeExporter]

I guess I have to make TSVN read the custom error message if there is one.

Original comment by tortoisesvn on 6 Oct 2009 at 6:53

[GoogleCodeExporter]

> if there is one
I believe there should be one. The .NET runtime will convert exception objects 
into 
COM error objects that hold the exception message.

> make TSVN read the custom error message 

You will need to use IErrorInfo:
http://msdn.microsoft.com/en-us/library/ms221510.aspx

I have attached some demo C++ code to verify this and which could help for 
TSVN. It 
instantiates Gurtle, calls GetCommitMessage with a bad parameter string and 
then 
tries to extract the error message string. If all goes well, this should 
read, "Parameter 'bad' is unknown." on output. The "Parameter 'bad' is 
unknown." 
string comes from the exception object thrown in .NET code!

P.S. Haven't done C++ in ages so go gentle on any oversights. :)

Original comment by azizatif on 6 Oct 2009 at 8:05

Attachments:

• gerrdemo.zip

[GoogleCodeExporter]

Thanks for your demo code. But I went (IMHO) a better way by creating a helper 
class
which does all the work for me.
Committed this for TSVN in r17379.

I'm leaving this issue open because the error message now shows:

Failed to start the issue tracker COM provider 'bugtraq:provider'.
Unknown error 0x80131500
Invalid project name.
Parameter name: value

The first two lines are from TSVN (the "unknown error" is the HRESULT I got 
back and
it's 'official' meaning).
But then the next two lines are from Gurtle: they don't really tell much. 
"Invalid
project name" is ok, but "Parameter name: value" doesn't mean anything to me. It
should read "Invalid project name: WRONGPROJECTNAME\nMake sure the project with 
that
name exists and that the project name is all in lowercase" or something like 
that. At
least something the user can recognize - IMHO "Parameter name: value" doesn't 
help at
all.

Original comment by tortoisesvn on 7 Oct 2009 at 7:15

[GoogleCodeExporter]

> went (IMHO) a better way by creating a helper class
> which does all the work for me.

Sure you went a better way. I could afford to go quick and dirty given that I 
was 
only doing a proof-of-concept. :)

> Unknown error 0x80131500

This doesn't make sense to display. The error is not unknown because it's right 
there on the next two lines even if they seem a bit meaningless right:

> Invalid project name.
> Parameter name: value

0x80131500 is a pretty general error code for someone throwing Exception from 
.NET 
code. The 0x13 identifies FACILITY_URT, where URT I believe means Universal 
Run-time 
and may have been an internal name for the .NET runtime. You can also see 
comment in 
CorError.h (from SSCLI) on line 13 that kind of confirms this:

http://tinyurl.com/corerror-h-ln13

13: FACILITY_URT is defined as 0x13 (0x8013xxxx). The facility range is reserved
14: for the .NET Framework SDK teams.

Further down in the same header file, it says "0x15yy for BCL". I would 
therefore 
show 0x80131500 but not the "Unknown error" unless you cannot get further info 
out 
IErrorInfo. Talking of IErrorInfo and TSVN@17379, you should also be checking 
with 
ISupportErrorInfo before relying on the IErrorInfo object as shown in the demo 
code. 
It would be more technically correct per COM protcol and history.

> But then the next two lines are from Gurtle: 
>they don't really tell much.

Agreed and good suggestions for the text.

Original comment by azizatif on 7 Oct 2009 at 9:20

[GoogleCodeExporter]

> 0x80131500 is a pretty general error code for someone throwing Exception from 
.NET 
> code. The 0x13 identifies FACILITY_URT, where URT I believe means Universal 
> Run-time 
> and may have been an internal name for the .NET runtime. You can also see 
comment 
> in 
> CorError.h (from SSCLI) on line 13 that kind of confirms this:
>
> http://tinyurl.com/corerror-h-ln13
[snip]

Sure, those HRESULTS might be fixed now. But unless they're also fixed and 
officially
documented by MS, I'm not going to use those.
Also, I don't want to hard code the HRESULTS to error strings if the API doesn't
return the right error string for those.

The error string TSVN shows now is returned by the Windows API that way, so I'm
showing that string (even though it could be a better (defined) error string).

> IErrorInfo. Talking of IErrorInfo and TSVN@17379, you should also be checking 
with 
> ISupportErrorInfo before relying on the IErrorInfo object as shown in the 
demo 
> code. 
> It would be more technically correct per COM protcol and history.

Not really: I'm getting the IErrorInfo with the GetErrorInfo() API. There's no 
other
interface to first query for ISupportErrorInfo first.

Original comment by tortoisesvn on 8 Oct 2009 at 5:20

[GoogleCodeExporter]

> I don't want to hard code the HRESULTS to error strings 

I think there's a misunderstanding. I'm not suggesting any hardwiring, but 
rather 
this:

> I would therefore show 0x80131500 but not the "Unknown error" 
> unless you cannot get further info out IErrorInfo.

I have a more fundamental question here about the error UI policy between the 
plug-
in and TSVN. Who is responsible for displaying an error UI? Should a plug-in 
always 
throw an error assuming TSVN will show the dialog box with the error 
information 
obtained via IErrorInfo or is that the plug-in's responsibility? The former 
approach 
seems better because it also tells TSVN that an operation failed. Moreover, not 
every plug-in has to provide their own error UI. Thoughts?

> There's no other
> interface to first query for ISupportErrorInfo 

You query the plug-in for ISupportErrorInfo via regular QueryInterface.

Original comment by azizatif on 8 Oct 2009 at 8:45

[GoogleCodeExporter]

The problem is that a lot of COM objects don't provide the full error info in 
the
description but rely on the error message from the HRESULT, and then only 
provide
additional information via the IErrorInfo description.
Since TSVN can't know what the COM object provides in the error description, 
I'd like
to show the error string from the HRESULT anyway.

About the error UI policy:
I would say that a plugin should show its own error dialogs if possible. That 
way it
can also provide a link to some help file or other documentation on how to fix 
the
error condition.
Only in cases where the plugin can't show an UI or doesn't has an UI, it should 
rely
on the COM error mechanism.

Original comment by tortoisesvn on 9 Oct 2009 at 5:21

[GoogleCodeExporter]

> TSVN can't know what the COM object provides in the error description

That is precisely the purpose of ISupportErrorInfo, to let you find out if, at 
least, the COM object claims supporting rich error information or not.

I understand the if the plug-in can show the UI then it has some additional 
control 
over how it can assist the user with a problem. However, how would TSVN ever 
know if 
the operation succeeded or failed? Maybe this is not a problem today with the 
current set of methods on IBugTraqProvider2 but the policy could bite back in 
the 
future. If Gurtle shows an error dialog due to invalid parameters and then goes 
on 
to return an HRESULT indicating failure to TSVN then won't that annoy the use 
with 
yet another dialog box with a more basic and possibly cryptic error dialog?

Original comment by azizatif on 9 Oct 2009 at 10:32

[GoogleCodeExporter]

ISupportErrorInfo only tells you *that* the object provides rich error 
information,
not *what*.
So for example, if a COM object returns E_INVALIDARG, the error string would say
something like "invalid parameter". But the COM object could then only provide
*additional* info in the error description like "the host does not exist". In 
such a
case, it's necessary to show *both* strings (error and description) to get all
information.

Of course, a plugin should only show its own error dialog if it can still do its
operation, maybe only partially. For example, if some information is missing 
(e.g.,
the project name) Gurtle could show an error dialog and let the user specify the
project name. If the user then clicks on a cancel button and doesn't specify a
project name, then it can throw an exception back to TSVN and TSVN will then 
show the
error too.

Original comment by tortoisesvn on 10 Oct 2009 at 6:20

[GoogleCodeExporter]

> if a COM object returns E_INVALIDARG...
> ...additional info in the error description like 
> "the host does not exist".

Your example is correct but we're going off the wrong end. Irrespective of 
whether 
you wish to show both errors or not, I was pointing out that it is incorrect to 
rely 
on GetErrorInfo in the first place if you don't ask the COM object's interface 
whether it supports rich error information or not.

> Gurtle could show an error dialog and let the user specify the
> project name.

I would agree if the parameters could be updated once user had provided a fix. 
During issue selection, however, the fixed project name would not be saved by 
TSVN. 
In fact, this raises another question. Shouldn't TSVN be calling 
ValidateParameters 
prior to any interface member also accepting parameters? The responsibilites 
are not 
very clear around these areas. 

May be this should've been a thread on the TSVN discussion group as it would 
apply 
to any plug-in but I'm trying to understand whether the current behavior to 
throw an 
error is not entirely incorrect. The error description does after all state 
clearly, "Invalid project name". I agree though that the, "Parameter name: 
value" 
line reads odd and may be the only bit that needs review here.

Original comment by azizatif on 13 Oct 2009 at 9:48

[GoogleCodeExporter]

> Your example is correct but we're going off the wrong end. Irrespective of 
whether 
> you wish to show both errors or not, I was pointing out that it is incorrect 
to rely 
> on GetErrorInfo in the first place if you don't ask the COM object's 
interface 
> whether it supports rich error information or not.

It's not a requirement to first call InterfaceSupportsErrorInfo(). This is only
needed in case you have multiple interfaces from the same object and you want 
to make
sure you ask the right one for the error. That's because IErrorInfo clears the 
error,
so in such a case you don't want to ask the wrong one for that interface and
accidentally clear the error.

It's enough to ask for the IErrorInfo interface and check if that interface 
exists if
we just want to show the last error and don't intend to pass the error on (TSVN 
isn't
a COM object itself, so it can't pass the error on).

Calling ValidateParameters() before every function call is of course possible, 
but do
we really want to do that? The plugin should handle situations where those 
parameters
are wrong itself in a nice way. See also issue #46 for why even empty or wrong
parameters can be 'fixed' in some functions.

Original comment by tortoisesvn on 14 Oct 2009 at 6:43