Don't return generic NS_ERROR_FAILURE error code from nsLocalFile methods
Categories
(Core :: XPCOM, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox86 | --- | fixed |
People
(Reporter: sg, Assigned: sg)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Some nsLocalFile methods, including IsDirectory
and GetDirectoryEntries
return a generic NS_ERROR_FAILURE
error code on Windows, presumably originated from an unmapped Windows error code from https://searchfox.org/mozilla-central/rev/692a10e624a02fe79bf45046b586e50ea0ff17f1/xpcom/io/nsLocalFileWin.cpp#293. These errors currently cause a significant number of storage initialization failures as described in Bug 1482662.
To understand the cause of these issues, it would be helpful to map these to a value that allows to identify the original WIndows error code(s).
Assignee | ||
Comment 1•4 years ago
|
||
In case there's no specific guess which Windows error code this might involve, we could add temporary telemetry that reports unmapped error codes. Nika, WDYT?
Comment 2•4 years ago
|
||
Using telemetry to report on unknown error codes seems like it could be reasonable.
The full set of error codes returnable by win32 APIs is very large (https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes), and so it is likely unreasonable for us to enumerate them all explicitly, but if there are specific high-frequency errors which we are not handling, we can definitely add them to the conversion function.
Comment 3•4 years ago
|
||
How about using NS_ERROR_GENERATE_FAILURE(FACILITY_WIN32 - NS_ERROR_MODULE_BASE_OFFSET, aWinErr)
? This is compatible with HRESULT_FROM_WIN32
.
Assignee | ||
Comment 4•4 years ago
|
||
(In reply to Nika Layzell [:nika] (ni? for response) from comment #2)
Using telemetry to report on unknown error codes seems like it could be reasonable.
Ok. Unless we implement emk's suggestion, I can take care of that.
The full set of error codes returnable by win32 APIs is very large (https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes), and so it is likely unreasonable for us to enumerate them all explicitly,
Sure.
(In reply to Masatoshi Kimura [:emk] from comment #3)
How about using
NS_ERROR_GENERATE_FAILURE(FACILITY_WIN32 - NS_ERROR_MODULE_BASE_OFFSET, aWinErr)
? This is compatible withHRESULT_FROM_WIN32
.
Interesting, I wasn't aware this option exists! That doesn't seem to be used anywhere else in the codebase though? Nika, what do you think about that, is that an option?
Comment 5•4 years ago
|
||
The only other place we use NS_ERROR_GENERATE_FAILURE
is wrapping NSS errors into XPCOM errors, like we do here: https://searchfox.org/mozilla-central/rev/03224522336f60a1a61a87e1fcd4feb0a0315a9b/security/manager/ssl/NSSErrorsService.cpp#81. This dynamic error logic is a bit unfortunate, as it means that our nsresult
enum doesn't exhaustively list all possible variants, but I suppose given that we already do it in one place it wouldn't be the worst thing to also do it here, at least on a temporary basis.
I would prefer not to use FACILITY_WIN32 - NS_ERROR_MODULE_BASE_OFFSET
as the module, as we don't need to preserve nsresult
compatibility with HRESULT
here, and I would prefer to specify an explicit NS_ERROR_MODULE
through ErrorList.py
which is known to our other language bindings as well.
Assignee | ||
Comment 6•4 years ago
|
||
Updated•4 years ago
|
Comment 8•4 years ago
|
||
Is there a canonical nsresult translation site that should potentially be updated? Ex: https://www.oxymoronical.com/nsresult/ (but it doesn't understand modules, I think? Looks like it just consults https://www.oxymoronical.com/nsresult/js/results.json)
Comment 9•4 years ago
|
||
bugherder |
Comment 10•4 years ago
|
||
(In reply to Andrew Sutherland [:asuth] (he/him) from comment #8)
Is there a canonical nsresult translation site that should potentially be updated? Ex: https://www.oxymoronical.com/nsresult/ (but it doesn't understand modules, I think? Looks like it just consults https://www.oxymoronical.com/nsresult/js/results.json)
I've filed bug 1688671 on making searchfox support error results. And I think it turns out that https://james-ross.co.uk/mozilla/misc/nserror is better than the above.
Description
•