On Mon, Mar 19, 2012 at 04:48, Filipe David Manana <[email protected]>wrote:
> Any special reason why couch_log and couch_file don't get the same > treatment (properly formatted error message) as couch_config and > couch_config_writer? > Hmm. I think maybe I should add it to couch_file because that makes a ton of sense. At the time, I was thinking I would just let it bubble and it would be caught above, as it certainly is in many cases. For couch_log, how can we log anything? It felt futile to try to invoke log calls when couch_log fails to start. > > Seems like a small (incidental) regression to me. > > thanks > > On Mon, Mar 19, 2012 at 3:56 AM, <[email protected]> wrote: > > remove unnecessary eaccess special casing > > > > It's better to let these errors bubble and/or not give them special > > treatment when file:format_error/1 can do a better job of describing > > the failure. > > > > > > Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo > > Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/48371335 > > Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/48371335 > > Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/48371335 > > > > Branch: refs/heads/COUCHDB-1445 > > Commit: 483713352cb397510111798c4b076afad83f6c46 > > Parents: 29eac04 > > Author: Randall Leeds <[email protected]> > > Authored: Sun Mar 18 20:19:27 2012 -0700 > > Committer: Randall Leeds <[email protected]> > > Committed: Sun Mar 18 20:41:42 2012 -0700 > > > > ---------------------------------------------------------------------- > > src/couchdb/couch_config.erl | 11 ++++------- > > src/couchdb/couch_config_writer.erl | 8 +++++--- > > src/couchdb/couch_file.erl | 5 +---- > > src/couchdb/couch_log.erl | 2 -- > > src/couchdb/couch_server_sup.erl | 14 +++++++------- > > 5 files changed, 17 insertions(+), 23 deletions(-) > > ---------------------------------------------------------------------- > > > > > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_config.erl > > ---------------------------------------------------------------------- > > diff --git a/src/couchdb/couch_config.erl b/src/couchdb/couch_config.erl > > index f669853..44a102d 100644 > > --- a/src/couchdb/couch_config.erl > > +++ b/src/couchdb/couch_config.erl > > @@ -187,13 +187,10 @@ parse_ini_file(IniFile) -> > > case file:read_file(IniFilename) of > > {ok, IniBin0} -> > > IniBin0; > > - {error, eacces} -> > > - throw({file_permission_error, IniFile}); > > - {error, enoent} -> > > - Fmt = "Couldn't find server configuration file ~s.", > > - Msg = ?l2b(io_lib:format(Fmt, [IniFilename])), > > - ?LOG_ERROR("~s~n", [Msg]), > > - throw({startup_error, Msg}) > > + {error, Reason} = Error -> > > + ?LOG_ERROR("Couldn't read server configuration file ~s: ~s", > > + [IniFilename, file:format_error(Reason)]), > > + throw(Error) > > end, > > > > Lines = re:split(IniBin, "\r\n|\n|\r|\032", [{return, list}]), > > > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_config_writer.erl > > ---------------------------------------------------------------------- > > diff --git a/src/couchdb/couch_config_writer.erl > b/src/couchdb/couch_config_writer.erl > > index decd269..3a65c37 100644 > > --- a/src/couchdb/couch_config_writer.erl > > +++ b/src/couchdb/couch_config_writer.erl > > @@ -22,6 +22,8 @@ > > > > -export([save_to_file/2]). > > > > +-include("couch_db.hrl"). > > + > > %% @spec save_to_file( > > %% Config::{{Section::string(), Option::string()}, > Value::string()}, > > %% File::filename()) -> ok > > @@ -38,9 +40,9 @@ save_to_file({{Section, Key}, Value}, File) -> > > case file:write_file(File, NewFileContents) of > > ok -> > > ok; > > - {error, eacces} -> > > - {file_permission_error, File}; > > - Error -> > > + {error, Reason} = Error -> > > + ?LOG_ERROR("Couldn't write config file ~s: ~s", > > + [File, file:format_error(Reason)]), > > Error > > end. > > > > > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_file.erl > > ---------------------------------------------------------------------- > > diff --git a/src/couchdb/couch_file.erl b/src/couchdb/couch_file.erl > > index e195db0..5e476af 100644 > > --- a/src/couchdb/couch_file.erl > > +++ b/src/couchdb/couch_file.erl > > @@ -58,10 +58,7 @@ open(Filepath, Options) -> > > {trap_exit, true} -> receive {'EXIT', Pid, _} -> ok end; > > {trap_exit, false} -> ok > > end, > > - case Error of > > - {error, eacces} -> {file_permission_error, Filepath}; > > - _ -> Error > > - end > > + Error > > end; > > Error -> > > Error > > > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_log.erl > > ---------------------------------------------------------------------- > > diff --git a/src/couchdb/couch_log.erl b/src/couchdb/couch_log.erl > > index 8e24cab..7fb95a7 100644 > > --- a/src/couchdb/couch_log.erl > > +++ b/src/couchdb/couch_log.erl > > @@ -89,8 +89,6 @@ init([]) -> > > case file:open(Filename, [append]) of > > {ok, Fd} -> > > {ok, #state{fd = Fd, level = Level, sasl = Sasl}}; > > - {error, eacces} -> > > - {stop, {file_permission_error, Filename}}; > > Error -> > > {stop, Error} > > end. > > > > > http://git-wip-us.apache.org/repos/asf/couchdb/blob/48371335/src/couchdb/couch_server_sup.erl > > ---------------------------------------------------------------------- > > diff --git a/src/couchdb/couch_server_sup.erl > b/src/couchdb/couch_server_sup.erl > > index 7baede3..be3c3a3 100644 > > --- a/src/couchdb/couch_server_sup.erl > > +++ b/src/couchdb/couch_server_sup.erl > > @@ -46,7 +46,9 @@ start_server(IniFiles) -> > > {ok, [PidFile]} -> > > case file:write_file(PidFile, os:getpid()) of > > ok -> ok; > > - Error -> io:format("Failed to write PID file ~s, error: ~p", > [PidFile, Error]) > > + {error, Reason} -> > > + io:format("Failed to write PID file ~s: ~s", > > + [PidFile, file:format_error(Reason)]) > > end; > > _ -> ok > > end, > > @@ -121,12 +123,10 @@ start_server(IniFiles) -> > > end end || Uri <- Uris], > > case file:write_file(UriFile, Lines) of > > ok -> ok; > > - {error, eacces} -> > > - ?LOG_ERROR("Permission error when writing to URI file ~s", > [UriFile]), > > - throw({file_permission_error, UriFile}); > > - Error2 -> > > - ?LOG_ERROR("Failed to write to URI file ~s: ~p~n", > [UriFile, Error2]), > > - throw(Error2) > > + {error, Reason2} = Error -> > > + ?LOG_ERROR("Failed to write to URI file ~s: ~s", > > + [UriFile, file:format_error(Reason2)]), > > + throw(Error) > > end > > end, > > > > > > > > -- > Filipe David Manana, > > "Reasonable men adapt themselves to the world. > Unreasonable men adapt the world to themselves. > That's why all progress depends on unreasonable men." >
