fs: fix misplaced errors in fs.symlinkSync

[joyeecheung]

The ctx.error is supposed to be handled in fs.readlinkSync,
but was handled in fs.symlinkSync by mistake.

Also fix the error number check in readlink to be consistent
with SYNC_CALL.

Refs: #18348

This is an oversight of #18348, the error handling was supposed to be in fs.readlinkSync instead of fs.symlinkSync. I couldn't come up with a test because these errors cannot be triggered easily:

node/src/string_bytes.cc

Lines 41 to 51 in 1286923

	#define SB_MALLOC_FAILED_ERROR \
	v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
	#define SB_STRING_TOO_LONG_ERROR \
	v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
	#define SB_BUFFER_CREATION_ERROR \
	v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))
	#define SB_BUFFER_SIZE_EXCEEDED_ERROR \
	v8::Exception::Error(OneByteString(isolate, "\"toString()\" failed"))

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

fs

[maclover7]

Should we add a regression test here? Otherwise LGTM

[joyeecheung]

@maclover7 The only way not to introduce more flakes that I can think of is monkey-patching the binding so that it returns ctx with an error...not a big fan of this way of testing but I can add that

[maclover7]

@joyeecheung Hmm, I may be oversimplifying / not completely understanding what's being changed here, but couldn't there be a test that checks the error stacktrace to make sure everything that's there is there? That seems like what the extra conditional provides

[joyeecheung]

@maclover7 The ctx.error is only triggered by the cases described in the OP, e.g. when malloc fails or the buffer to encode is too big. Normal syscall errors don't go there

[BridgeAR]

I personally think monkey patching to test this is appropriate but I am ok with landing this without a test as well.

[BridgeAR]

@joyeecheung can you please always run a CI after creating a PR? :-)

[BridgeAR]

CI https://ci.nodejs.org/job/node-test-pull-request/12974/

[joyeecheung]

Rebased. CI: https://ci.nodejs.org/job/node-test-pull-request/13033/

[joyeecheung]

Landed in b1c6ecb, thanks!

[MylesBorins]

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.