Closed Bug 1077078 Opened 10 years ago Closed 10 years ago

Avoid debug asserts in readURI when trying to load a non-existent resource URI

Categories

(Add-on SDK Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla35

People

(Reporter: mossop, Assigned: mossop)

References

Details

Attachments

(1 file, 1 obsolete file)

993 bytes, patch
mcmanus
: review+
Details | Diff | Splinter Review
NetUtil.newChannel asserts if attempting to access a resource URI that doesn't have a mapping. NetUtil.newChannel is just a shortcut method though, if we do the few steps ourselves instead we avoid spamming the console with errors in debug builds.
Attached file pull request (obsolete) —
This is perhaps a quicker way to be able to get mochitest-jetpack moving again.
Attachment #8499121 - Flags: review?(evold)
Comment on attachment 8499121 [details] [review]
pull request

Looks good, I think we can write a test for this though no?
Attachment #8499121 - Flags: review?(evold) → feedback+
(In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> Comment on attachment 8499121 [details] [review]
> pull request
> 
> Looks good, I think we can write a test for this though no?

I don't know how, the spam is written direct to the error console. It doesn't go through the regular console service so there is no easy way in process to catch it.
(In reply to Dave Townsend [:mossop] from comment #3)
> (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> > Comment on attachment 8499121 [details] [review]
> > pull request
> > 
> > Looks good, I think we can write a test for this though no?
> 
> I don't know how, the spam is written direct to the error console. It
> doesn't go through the regular console service so there is no easy way in
> process to catch it.

Alright,(In reply to Dave Townsend [:mossop] from comment #3)
> (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> > Comment on attachment 8499121 [details] [review]
> > pull request
> > 
> > Looks good, I think we can write a test for this though no?
> 
> I don't know how, the spam is written direct to the error console. It
> doesn't go through the regular console service so there is no easy way in
> process to catch it.

can we not use nsIConsoleService's registerListener method ?
Flags: needinfo?(dtownsend+bugmail)
(In reply to Erik Vold [:erikvold] (work week -> pto) from comment #4)
> (In reply to Dave Townsend [:mossop] from comment #3)
> > (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> > > Comment on attachment 8499121 [details] [review]
> > > pull request
> > > 
> > > Looks good, I think we can write a test for this though no?
> > 
> > I don't know how, the spam is written direct to the error console. It
> > doesn't go through the regular console service so there is no easy way in
> > process to catch it.
> 
> Alright,(In reply to Dave Townsend [:mossop] from comment #3)
> > (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> > > Comment on attachment 8499121 [details] [review]
> > > pull request
> > > 
> > > Looks good, I think we can write a test for this though no?
> > 
> > I don't know how, the spam is written direct to the error console. It
> > doesn't go through the regular console service so there is no easy way in
> > process to catch it.
> 
> can we not use nsIConsoleService's registerListener method ?

I totally typed that wrong. The spam is written to stderr. nsIConsoleService knows nothing about it.
Flags: needinfo?(dtownsend+bugmail)
(In reply to Dave Townsend [:mossop] from comment #5)
> (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #4)
> > (In reply to Dave Townsend [:mossop] from comment #3)
> > > (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> > > > Comment on attachment 8499121 [details] [review]
> > > > pull request
> > > > 
> > > > Looks good, I think we can write a test for this though no?
> > > 
> > > I don't know how, the spam is written direct to the error console. It
> > > doesn't go through the regular console service so there is no easy way in
> > > process to catch it.
> > 
> > Alright,(In reply to Dave Townsend [:mossop] from comment #3)
> > > (In reply to Erik Vold [:erikvold] (work week -> pto) from comment #2)
> > > > Comment on attachment 8499121 [details] [review]
> > > > pull request
> > > > 
> > > > Looks good, I think we can write a test for this though no?
> > > 
> > > I don't know how, the spam is written direct to the error console. It
> > > doesn't go through the regular console service so there is no easy way in
> > > process to catch it.
> > 
> > can we not use nsIConsoleService's registerListener method ?
> 
> I totally typed that wrong. The spam is written to stderr. nsIConsoleService
> knows nothing about it.

what is the error message?
Flags: needinfo?(dtownsend+bugmail)
[18868] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file /Users/dave/mozilla/source/trunk/netwerk/base/src/nsIOService.cpp, line 598

It's ultimately logged to the output here: http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsDebugImpl.cpp#390
Flags: needinfo?(dtownsend+bugmail)
Ah thanks,

I was talking to Jim Blandy and he was saying that we should just replace 

http://dxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsIOService.cpp#598

NS_ENSURE_SUCCESS(rv, rv);


with 

http://dxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsIOService.cpp#590-591

if (NS_FAILED(rv))
    return rv;

since it's the same thing without the stderr print.
(In reply to Erik Vold [:erikvold] (work week -> pto) from comment #8)
> Ah thanks,
> 
> I was talking to Jim Blandy and he was saying that we should just replace 
> 
> http://dxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsIOService.
> cpp#598
> 
> NS_ENSURE_SUCCESS(rv, rv);
> 
> 
> with 
> 
> http://dxr.mozilla.org/mozilla-central/source/netwerk/base/src/nsIOService.
> cpp#590-591
> 
> if (NS_FAILED(rv))
>     return rv;
> 
> since it's the same thing without the stderr print.

Patrick, would you accept a patch to do that?
Flags: needinfo?(mcmanus)
(In reply to Dave Townsend [:mossop] from comment #9)
>
> Patrick, would you accept a patch to do that?

sure
Flags: needinfo?(mcmanus)
Attached patch patchSplinter Review
Here you go then
Attachment #8499121 - Attachment is obsolete: true
Attachment #8499281 - Flags: review?(mcmanus)
Attachment #8499281 - Flags: review?(mcmanus) → review+
https://hg.mozilla.org/mozilla-central/rev/4f2d51895db5
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.